- 27 Jun, 2019 27 commits
-
-
Thomas Zimmermann authored
The mgag200 driver's struct mga_framebuffer is a buffer object with GEM interface. There are already GEM framebuffer helpers that implement the same functionality. Convert mgag200 to these. Signed-off-by:
Thomas Zimmermann <tzimmermann@suse.de> Acked-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190627080909.30471-1-tzimmermann@suse.de
-
Andrey Smirnov authored
Transfer size of zero means a request to do an address-only transfer. Since the HW support this, we probably shouldn't be just ignoring such requests. While at it allow DP_AUX_I2C_MOT flag to pass through, since it is supported by the HW as well. Signed-off-by:
Andrey Smirnov <andrew.smirnov@gmail.com> Reviewed-by:
Andrzej Hajda <a.hajda@samsung.com> Reviewed-by:
Tomi Valkeinen <tomi.valkeinen@ti.com> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> Cc: Andrey Gusakov <andrey.gusakov@cogentembedded.com> Cc: Philipp Zabel <p.zabel@pengutronix.de> Cc: Cory Tusar <cory.tusar@zii.aero> Cc: Chris Healy <cphealy@gmail.com> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org Signed-off-by:
-
Andrey Smirnov authored
We don't need 8 byte array, DP_LINK_STATUS_SIZE (6) should be enough. This also gets rid of a magic number as a bonus. Signed-off-by:
-
Andrey Smirnov authored
tc_get_display_props() never reads more than a byte via AUX, so there's no need to reserve 8 for that purpose. No function change intended. Signed-off-by:
-
Andrey Smirnov authored
We never pass anything but 100 as timeout_ms to tc_aux_wait_busy(), so we may as well hardcode that value and simplify function's signature. Signed-off-by:
-
Andrey Smirnov authored
tc_wait_pll_lock() is always called as a follow-up for updating PLLUPDATE and PLLEN bit of a given PLL control register. To simplify things, merge the two operation into a single helper function tc_pllupdate() and convert the rest of the code to use it. No functional change intended. Signed-off-by:
Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by:
-
Andrey Smirnov authored
Move common code converting clock rate to an appropriate constant and configuring SYS_PLLPARAM register into a separate routine and convert the rest of the code to use it. No functional change intended. Signed-off-by:
-
Andrey Smirnov authored
Don't assume that requested data transfer size is the same as amount of data that was transferred. Change the code to get that information from DP0_AUXSTATUS instead. Since the check for AUX_BUSY in tc_aux_get_status() is pointless (it will always called after tc_aux_wait_busy()) and there's only one user of it, inline its code into tc_aux_transfer() instead of trying to accommodate the change above. Signed-off-by:
-
Andrey Smirnov authored
According to the datasheet tc358767 can transfer up to 16 bytes via its AUX channel, so the artificial limit of 8 appears to be too low. However only up to 15-bytes seem to be actually supported and trying to use 16-byte transfers results in transfers failing sporadically (with bogus status in case of I2C transfers), so limit it to 15. Signed-off-by:
-
Andrey Smirnov authored
Simplify AUX data write by dropping index arithmetic and shifting and replacing it with a call to a helper function that does two things: 1. Copies user-provided data into a write buffer 2. Transfers contents of the write buffer to up to 4 32-bit registers on the chip Note that separate data endianness fix: tmp = (tmp << 8) | buf[i]; that was reserved for DP_AUX_I2C_WRITE looks really strange, since it will place data differently depending on the passed user-data size. E.g. for a write of 1 byte, data transferred to the chip would look like: [byte0] [dummy1] [dummy2] [dummy3] whereas for a write of 4 bytes we'd get: [byte3] [byte2] [byte1] [byte0] Since there's no indication in the datasheet that I2C write buffer should be treated differently than AUX write buffer and no comment in the original code explaining why it was done this way, that special I2C write buffer transformation was dropped in this patch. Signed-off-by: -
Andrey Smirnov authored
Simplify AUX data read by removing index arithmetic and shifting with a helper function that does two things: 1. Fetch data from up to 4 32-bit registers from the chip 2. Copy read data into user provided array. Signed-off-by: -
Andrey Smirnov authored
A very unfortunate aspect of tc_write()/tc_read() macro helpers is that they capture quite a bit of context around them and thus require the caller to have magic variables 'ret' and 'tc' as well as label 'err'. That makes a number of code paths rather counter-intuitive and somewhat clunky, for example tc_stream_clock_calc() ends up being like this: int ret; tc_write(DP0_VIDMNGEN1, 32768); return 0; err: return ret; which is rather surprising when you read the code for the first time. Since those helpers arguably aren't really saving that much code and there's no way of fixing them without making them too verbose to be worth it change the driver code to not use them at all. Signed-off-by:
-
Andrey Smirnov authored
Simplify tc_set_video_mode() by replacing explicit shifting using macros from <linux/bitfield.h>. No functional change intended. Signed-off-by:
-
Andrey Smirnov authored
Replace explicit polling in tc_link_training() with equivalent call to tc_poll_timeout() for simplicity. No functional change intended (not including slightly altered debug output). Signed-off-by:
-
Andrey Smirnov authored
Replace explicit polling loop with equivalent call to tc_poll_timeout() for brevity. No functional change intended. Signed-off-by:
-
Andrey Smirnov authored
Implementation of tc_poll_timeout() is almost a 100% copy-and-paste of the code for regmap_read_poll_timeout(). Replace copied code with a call to the original. While at it change tc_poll_timeout to accept "struct tc_data *" instead of "struct regmap *" for brevity. No functional change intended. Signed-off-by:
-
Rob Herring authored
The panel bindings are outside the scope of the Rockchip LVDS interface binding. The references are about to change too, so rather than update them just drop the section on the panel bindings. Cc: Sandy Huang <hjc@rock-chips.com> Cc: "Heiko Stübner" <heiko@sntech.de> Cc: Maxime Ripard <maxime.ripard@bootlin.com> Cc: dri-devel@lists.freedesktop.org Cc: linux-rockchip@lists.infradead.org Signed-off-by:
Rob Herring <robh@kernel.org> Signed-off-by:
Heiko Stuebner <heiko@sntech.de> Link: https://patchwork.freedesktop.org/patch/msgid/20190624215649.8939-2-robh@kernel.org
-
Colin Ian King authored
There is an if block that is missing the { } curly brackets. Add these in. Addresses-Coverity: ("Structurally dead code") Fixes: 94dc57b1 ("drm/mgag200: Rewrite cursor handling") Signed-off-by:Colin Ian King <colin.king@canonical.com> Signed-off-by:
-
Daniel Vetter authored
We're now guaranteed to no longer race against prepare_fb/cleanup_fb, which means we can access ->vaddr without having to hold a lock. Before the previous patches it was fairly easy to observe the cursor ->vaddr being invalid, but that's now gone, so we can upgrade to a full WARN_ON. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
Daniel Vetter <daniel.vetter@intel.com> Reviewed-by:
Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Tested-by:
-
Daniel Vetter authored
The crc computation worker needs to be able to get at some data structures and framebuffer mappings, while potentially more atomic updates are going on. The solution thus far is to copy relevant bits around, but that's very tedious. Here's a new approach, which tries to be more clever, but relies on a few not-so-obvious things: - crtc_state is always updated when a plane_state changes. Therefore we can just stuff plane_state pointers into a crtc_state. That solves the problem of easily getting at the needed plane_states. - with the flushing changes from previous patches the above also holds without races due to the next atomic update being a bit eager with cleaning up pending work - we always wait for all crc work items to complete before unmapping framebuffers. - we also need to make sure that the hrtimer fires off the right worker. Keep a new distinct crc_state pointer, under the vkms_output->lock protection for this. Note that crtc->state is updated very early in the atomic commit, way before we arm the vblank event - the vblank event should always match the buffers we use to compute the crc. This also solves an issue in the hrtimer, where we've accessed drm_crtc->state without holding the right locks (we held none - oops). - in the worker itself we can then just access the plane states we need, again solving a bunch of ordering and locking issues. Accessing plane->state requires locks, accessing the private vkms_crtc_state->active_planes pointer only requires that the memory doesn't get freed too early. The idea behind vkms_crtc_state->active_planes is that this would contain all visible planes, in z-order, as a first step towards a more generic blending implementation. Note that this patch also fixes races between prepare_fb/cleanup_fb and the crc worker accessing ->vaddr. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Daniel Vetter authored
irqs are already off. Signed-off-by:
-
Daniel Vetter authored
The crc core code can cope with some late crc, the race is kinda unavoidable. So no need to flush pending workers, they'll complete in time. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Daniel Vetter authored
Currently, we flush pending CRC workers very late in the commit flow, when we destroy all the old crtc states. Unfortunately, at that point, the framebuffers are already unpinned (and our vaddr possible gone), so this isn't good. Also, the plane_states we need might also already be cleaned up, since cleanup order of state structures isn't well defined. Fix this by waiting for all CRC workers of the old state to complete before we start any of the cleanup work. For correct ordering and avoiding races, we can only flush_work after drm_atomic_helper_wait_for_vblanks() since we know that all subsequent queue_work will be for the new state. Only once that's done is flush_work() useful, before that we might flush the work, and then right after the hrtimer that simulates vblank queues it again. Every time you have a flush_work before cleaning up the work structure, the following sequence must be obeyed, or it can go wrong: 1. Make sure no one else can re-queue the work anymore (in our case that's done by a combination of first updating output->crc_state and then waiting for the vblank to pass to make sure the hrtimer has noticed that change). 2. flush_work() 3. Actually clean up stuff (which isn't done here). Doing the flush_work before we even completed the output->state update, much less waited for the vblank to make sure that's happened, missed the point. Note that this is not yet race-free because of the hrtimer and crc worker look at the wrong state pointers, but that will be fixed in subsequent patches. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Daniel Vetter authored
Just prep work, more will be done here in following patches. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Daniel Vetter authored
Plus add a comment about what it actually protects. It's very little. Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Daniel Vetter authored
The worker is always in process context, no need for the _irqsafe version. Same for the set_source callback, that's only called from the debugfs handler in a syscall. Cc: Shayenne Moura <shayenneluzmoura@gmail.com> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Daniel Vetter authored
The issue we have is that the crc worker might fall behind. We've tried to handle this by tracking both the earliest frame for which it still needs to compute a crc, and the last one. Plus when the crtc_state changes, we have a new work item, which are all run in order due to the ordered workqueue we allocate for each vkms crtc. Trouble is there's been a few small issues in the current code: - we need to capture frame_end in the vblank hrtimer, not in the worker. The worker might run much later, and then we generate a lot of crc for which there's already a different worker queued up. - frame number might be 0, so create a new crc_pending boolean to track this without confusion. - we need to atomically grab frame_start/end and clear it, so do that all in one go. This is not going to create a new race, because if we race with the hrtimer then our work will be re-run. - only race that can happen is the following: 1. worker starts 2. hrtimer runs and updates frame_end 3. worker grabs frame_start/end, already reading the new frame_end, and clears crc_pending 4. hrtimer calls queue_work() 5. worker completes 6. worker gets re-run, crc_pending is false Explain this case a bit better by rewording the comment. v2: Demote warning level output to debug when we fail to requeue, this is expected under high load when the crc worker can't quite keep up. Cc: Shayenne Moura <shayenneluzmoura@gmail.com> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> Cc: Haneen Mohammed <hamohammed.sa@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
- 26 Jun, 2019 13 commits
-
-
Chris Wilson authored
We gracefully handle the caller specifying a zero range, so don't force them to special case that condition if it naturally falls out of their setup. What we don't check is if the end < start, so keep that as an assert for an illegal call. Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190626094330.3556-1-chris@chris-wilson.co.uk
-
Emil Velikov authored
The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: virtualization@lists.linux-foundation.org Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
Emil Velikov <emil.velikov@collabora.com> Acked-by:
Gerd Hoffmann <kraxel@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190527081741.14235-12-emil.l.velikov@gmail.com
-
Emil Velikov authored
The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Emil Velikov authored
The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Note: the outstanding DRM_AUTH instance is: - (badly coped) legacy DRI1 ioctl, which is a noop Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Emil Velikov authored
The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Note: the outstanding DRM_AUTH instance is: - legacy DRI1 ioctl, which is already neutered Cc: Ben Skeggs <bskeggs@redhat.com> Cc: nouveau@lists.freedesktop.org Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
Ben Skeggs <bskeggs@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190527081741.14235-8-emil.l.velikov@gmail.com
-
Emil Velikov authored
The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Rob Clark <robdclark@gmail.com> Cc: Sean Paul <sean@poorly.run> Cc: freedreno@lists.freedesktop.org Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Emil Velikov authored
The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Qiang Yu <yuq825@gmail.com> Cc: lima@lists.freedesktop.org Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
Qiang Yu <yuq825@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190527081741.14235-6-emil.l.velikov@gmail.com
-
Emil Velikov authored
The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Inki Dae <inki.dae@samsung.com> Cc: Joonyoung Shim <jy0922.shim@samsung.com> Cc: Seung-Woo Kim <sw0312.kim@samsung.com> Cc: Kyungmin Park <kyungmin.park@samsung.com> Cc: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> Signed-off-by:
Inki Dae <inki.dae@samsung.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190527081741.14235-4-emil.l.velikov@gmail.com
-
Emil Velikov authored
The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Christian Gmeiner <christian.gmeiner@gmail.com> Cc: etnaviv@lists.freedesktop.org Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
Christian Gmeiner <christian.gmeiner@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190527081741.14235-3-emil.l.velikov@gmail.com
-
Emil Velikov authored
Currently vmw_execbuf_ioctl() open-codes the permission checking, size extending and copying that is already done in core drm. Kill all the duplication, adding a few comments for clarity. Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> Cc: Thomas Hellstrom <thellstrom@vmware.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
Thomas Hellstrom <thellstrom@vmware.com> Reviewed-by:
-
Emil Velikov authored
According to the docs - prevents firstopen/lastclose races. Yet never used in practise. Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> Cc: Thomas Hellstrom <thellstrom@vmware.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Emil Velikov authored
Core DRM is safe when the callback is NULL. Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> Cc: Thomas Hellstrom <thellstrom@vmware.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by:
-
Emil Velikov authored
Cc: Ben Skeggs <bskeggs@redhat.com> Cc: nouveau@lists.freedesktop.org Signed-off-by:
-
