In fb_get_options(), always duplicate the returned option string and
transfer ownership of the memory to the function's caller.
Until now, only the global option string got duplicated and transferred
to the caller; the per-driver options were owned by fb_get_options().
In the end, it was impossible for the function's caller to detect if
it had to release the string's memory buffer. Hence, all calling drivers
leak the memory buffer. The leaks have existed ever since, but drivers
only call fb_get_option() once as part of module initialization. So the
amount of leaked memory is not significant.
Fix the semantics of fb_get_option() by unconditionally transferring
ownership of the memory buffer to the caller. Later patches can resolve
the memory leaks in the fbdev drivers.
Signed-off-by: Thomas Zimmermann <[email protected]>
Reviewed-by: Javier Martinez Canillas <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
This reverts commit ae71ab585c.
Commit ae71ab585c ("drm/vc4: hdmi: Enforce the minimum rate at
runtime_resume") was introduced to work around an issue partly due to
the clk-bcm2835 driver on the RaspberryPi0-3.
Since we're not using that driver for our HDMI clocks, we can now revert
it.
Reviewed-by: Javier Martinez Canillas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Maxime Ripard <[email protected]>
This reverts commit 3bc6a37f59.
Commit 3bc6a37f59 ("drm/vc4: hdmi: Fix HSM clock too low on Pi4") was
introduced to work around an issue partly due to the clk-bcm2835 driver
on the RaspberryPi0-3.
Since we're not using that driver for our HDMI clocks, we can now revert
that inelegant solution.
Reviewed-by: Javier Martinez Canillas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Maxime Ripard <[email protected]>
On the RaspberryPi0-3, the HSM clock was provided by the clk-bcm2835
driver, but on the Pi4 it was provided by the firmware through the
clk-raspberrypi driver.
The clk-bcm2835 driver registers the HSM clock using the
CLK_SET_RATE_GATE flag that prevents any modification to the rate while
the clock is active.
This meant that we needed to call clk_set_min_rate() before our call to
pm_runtime_resume_and_get() since our runtime_resume implementation
needs to enable the HSM clock for the HDMI controller registers to be
functional.
However, the HSM clock is part of the HDMI power domain which might not
be powered prior to the pm_runtime_resume_and_get() call, so we could
end up changing the rate of the HSM clock while its power domain was
disabled.
We recently changed the backing driver for the RaspberryPi0-3 to
clk-raspberrypi though, which doesn't have such restrictions. We can
thus move the clk_set_min_rate() after our call to runtime_resume and
avoid the access while the power domain is disabled.
Reviewed-by: Javier Martinez Canillas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Maxime Ripard <[email protected]>
The 120MHz value hardcoded in the call to max_t to compute the HSM rate
is defined in the driver as HSM_MIN_CLOCK_FREQ, let's switch to it so
that it's more readable.
Reviewed-by: Javier Martinez Canillas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Maxime Ripard <[email protected]>
v3: Fix vmw_user_bo_lookup which was also dropping the gem reference
before the kernel was done with buffer depending on userspace doing
the right thing. Same bug, different spot.
It is possible for userspace to predict the next buffer handle and
to destroy the buffer while it's still used by the kernel. Delay
dropping the internal reference on the buffers until kernel is done
with them.
Instead of immediately dropping the gem reference in vmw_user_bo_lookup
and vmw_gem_object_create_with_handle let the callers decide when they're
ready give the control back to userspace.
Also fixes the second usage of vmw_gem_object_create_with_handle in
vmwgfx_surface.c which wasn't grabbing an explicit reference
to the gem object which could have been destroyed by the userspace
on the owning surface at any point.
Signed-off-by: Zack Rusin <[email protected]>
Fixes: 8afa13a058 ("drm/vmwgfx: Implement DRIVER_GEM")
Reviewed-by: Martin Krastev <[email protected]>
Reviewed-by: Maaz Mombasawala <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
ttm_bo_init_reserved on failure puts the buffer object back which
causes it to be deleted, but kfree was still being called on the same
buffer in vmw_bo_create leading to a double free.
After the double free the vmw_gem_object_create_with_handle was
setting the gem function objects before checking the return status
of vmw_bo_create leading to null pointer access.
Fix the entire path by relaying on ttm_bo_init_reserved to delete the
buffer objects on failure and making sure the return status is checked
before setting the gem function objects on the buffer object.
Signed-off-by: Zack Rusin <[email protected]>
Fixes: 8afa13a058 ("drm/vmwgfx: Implement DRIVER_GEM")
Reviewed-by: Maaz Mombasawala <[email protected]>
Reviewed-by: Martin Krastev <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Various bits of the driver used raw ttm_buffer_object instead of the
driver specific vmw_bo object. All those places used to duplicate
the mapped bo caching policy of vmw_bo.
Instead of duplicating all of that code and special casing various
functions to work both with vmw_bo and raw ttm_buffer_object's unify
the buffer object handling code.
As part of that work fix the naming of bo's, e.g. insted of generic
backup use 'guest_memory' because that's what it really is.
All of it makes the driver easier to maintain and the code easier to
read. Saves 100+ loc as well.
Signed-off-by: Zack Rusin <[email protected]>
Reviewed-by: Martin Krastev <[email protected]>
Reviewed-by: Maaz Mombasawala <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Problem with explicit placement selection in vmwgfx is that by the time
the buffer object needs to be validated the information about which
placement was supposed to be used is lost. To workaround this the driver
had a bunch of state in various places e.g. as_mob or cpu_blit to
somehow convey the information on which placement was intended.
Fix it properly by allowing the buffer objects to hold their preferred
placement so it can be reused whenever needed. This makes the entire
validation pipeline a lot easier both to understand and maintain.
Signed-off-by: Zack Rusin <[email protected]>
Reviewed-by: Martin Krastev <[email protected]>
Reviewed-by: Maaz Mombasawala <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Only the legacy display unit requires pinning of the fb memory in vram.
Both the screen objects and screen targets can present from any buffer.
That makes the pinning abstraction pointless. Simplify all of the code
and move it to the legacy display unit, the only place that needs it.
Signed-off-by: Zack Rusin <[email protected]>
Reviewed-by: Martin Krastev <[email protected]>
Reviewed-by: Maaz Mombasawala <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
The rest of the drivers which are using ttm have mostly standardized on
driver_prefix_bo as the name for subclasses of the TTM buffer object.
Make vmwgfx match the rest of the drivers and follow the same naming
semantics.
This is especially clear given that the name of the file in which the
object was defined is vmw_bo.c.
Signed-off-by: Zack Rusin <[email protected]>
Reviewed-by: Martin Krastev <[email protected]>
Reviewed-by: Maaz Mombasawala <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Remove the explicit bo_free parameter which was switching between
vmw_bo_bo_free and vmw_gem_destroy which had exactly the same
implementation.
It makes no sense to keep parameter which is always the same, remove it
and all code referencing it. Instead use the vmw_bo_bo_free directly.
Signed-off-by: Zack Rusin <[email protected]>
Reviewed-by: Martin Krastev <[email protected]>
Reviewed-by: Maaz Mombasawala <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Before vmwgfx supported gem it needed to implement the entire mmap logic
explicitly. With GEM support that's not needed and the generic code
can be used by simply setting the vm_ops to vmwgfx specific ones on the
gem object itself.
Removes a lot of code from vmwgfx without any functional difference.
Signed-off-by: Zack Rusin <[email protected]>
Reviewed-by: Thomas Zimmermann <[email protected]>
Reviewed-by: Martin Krastev <[email protected]>
Reviewed-by: Maaz Mombasawala <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Other functions touching shmem->sgt take the pages lock, so do that here
too. drm_gem_shmem_get_pages() & co take the same lock, so move to the
_locked() variants to avoid recursive locking.
Discovered while auditing locking to write the Rust abstractions.
Fixes: 2194a63a81 ("drm: Add library for shmem backed GEM objects")
Fixes: 4fa3d66f13 ("drm/shmem: Do dma_unmap_sg before purging pages")
Signed-off-by: Asahi Lina <[email protected]>
Reviewed-by: Javier Martinez Canillas <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
vgem_fence_open() instantiates a mutex for a particular fence
instance, but never destroys it by calling mutex_destroy() in
vgem_fence_close().
So, add the missing mutex_destroy() to guarantee proper resource
destruction.
Fixes: 4077798484 ("drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)")
Signed-off-by: Maíra Canal <[email protected]>
Reviewed-by: Stanislaw Gruszka <[email protected]>
Signed-off-by: Maíra Canal <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Commit 15b9ca1641 ("drm: Config orientation property if panel provides
it") added a helper to set the panel orientation early but only
connected this for drm_bridge_connector, which constructs a panel bridge
with DRM_BRIDGE_ATTACH_NO_CONNECTOR and creates the connector itself.
When the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is not specified and the
panel_bridge creates its own connector the orientation is not set unless
the panel does it in .get_modes which is too late and leads to a warning
splat from __drm_mode_object_add() because the device is already
registered.
Call the necessary function to set add the orientation property when the
connector is created so that it is available before the device is
registered.
Signed-off-by: John Keeping <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
We have checks for this in the individual drivers move callback, but
it's probably better to generally forbid that on a higher level.
Also stops exporting ttm_resource_compat() since that's not necessary
any more after removing the extra checks in vmwgfx.
Signed-off-by: Christian König <[email protected]>
Reviewed-by: Matthew Auld <[email protected]>
Signed-off-by: Matthew Auld <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
In the next few patches, when initially creating a ttm BO, the
bo->resource is NULL, and the driver is then expected to handle the
initial dummy move. However, if this is created as a system resource
the first ttm_tt we create will always have the clear value set to
false. Previously the initial ttm_tt would be created in
ttm_bo_validate() with the clear parameter always set to true.
Signed-off-by: Matthew Auld <[email protected]>
Reviewed-by: Christian König <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Acked-by: Nirmoy Das <[email protected]>
Signed-off-by: Christian König <[email protected]>
In the near future TTM will have NULL bo->resource when the object is
initially created, plus after calling into pipeline-gutting. Try to
handle the remaining cases. In practice NULL bo->resource should be
taken to mean swapped-out or purged object.
v2 (Andrzej):
- Rather make i915_ttm_cpu_maps_iomem() return false with NULL
resource.
References: 516198d317 ("drm/i915: audit bo->resource usage v3")
Signed-off-by: Matthew Auld <[email protected]>
Cc: Nirmoy Das <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Nirmoy Das <[email protected]>
Reviewed-by: Andrzej Hajda <[email protected]>
Acked-by: Christian König <[email protected]>
Signed-off-by: Christian König <[email protected]>
The VPU_JSM_MSG_CONTEXT_DELETE will remove any resources associated
with the SSID, that included any blobs create by the user space
application.
The command can also remove doorbell registrations, but since this
does not work in HW scheduling case, we do not depend on this
capability and unregister the doorbells explicitly.
Signed-off-by: Andrzej Kacprowski <[email protected]>
Signed-off-by: Stanislaw Gruszka <[email protected]>
Reviewed-by: Jeffrey Hugo <[email protected]>
Signed-off-by: Jacek Lawrynowicz <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
The downstream code from which this was derived didn't ever run through
this 'switch' block with non-AFBC formats, but the upstream code does --
we use this function to probe whether a given format is supported.
Demote the warning to eliminate this sort of warning seen on every
boot:
[drm] unsupported AFBC format[3231564e]
And make it warn more than once, because if we *actually* care to see
what formats we're probing/rejecting and for what reasons, we probably
care about more than just the first message.
Drop the comment, because one of the two *is* commonly reachable.
And lastly, drop the unreachable return; we'd do better to let the
compiler complain if we start hitting this unexpectedly.
Signed-off-by: Brian Norris <[email protected]>
Signed-off-by: Heiko Stuebner <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/20221031101557.1.Ic1569d394173c1c3016142fee4bb87a09753db94@changeid
Commit 540b8f271e ("drm/rockchip: Embed drm_encoder into
rockchip_decoder") provides the means to pass the endpoint ID to the
VOP2 driver, which sets the interface MUX accordingly. However, this
step has not yet been carried out for the RGB output block. Embed the
drm_encoder structure into the rockchip_encoder structure and set the
endpoint ID correctly.
Signed-off-by: Michael Riesch <[email protected]>
Reviewed-by: Sascha Hauer <[email protected]>
Signed-off-by: Heiko Stuebner <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Thomas Zimmermann