We recently introduced a new mutex to protect concurrent execution of
ALSA and KMS hooks, and the concurrent access to some of vc4_hdmi
fields.
However, using it in the detect hook was creating a reentrency issue
with CEC code. Indeed, calling cec_s_phys_addr_from_edid from detect
might call the CEC adap_enable hook with the lock held, eventually
resulting in a deadlock.
Since we didn't really need to protect anything at the moment in the CEC
code, the decision was made to ignore the mutex in those CEC hooks,
working around the issue.
However, we can have the same thing happening if we end up triggering a
mode set from the detect callback, for example using
drm_atomic_helper_connector_hdmi_reset_link().
Since we don't really need to protect anything in detect either, let's
just drop the lock in detect, and add it again in CEC.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
drm_plane_state->src might be modified by the driver. This is done
e.g. in i915 driver when there is bigger framebuffer than the plane
and there is some offset within framebuffer. I915 driver calculates
separate offset and adjusts src rect coords to be relative to this
offset. Damage clips are still relative to original src coords
provided by user-space.
This patch ensures original coordinates provided by user-space are
used when initiliazing damage iterator.
Signed-off-by: Jouni Högander <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Mika Kahola <[email protected]>
Add drm_fb_build_fourcc_list() function that builds a list of supported
formats from native and emulated ones. Helpful for all drivers that do
format conversion as part of their plane updates. Update current caller.
v3:
* improve warnings on ignored formats (Sam)
v2:
* use u32 instead of uint32_t (Sam)
* print a warning if output array is too small (Sam)
* comment fixes (Sam)
Signed-off-by: Thomas Zimmermann <[email protected]>
Reviewed-by: Sam Ravnborg <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Add drm_crtc_helper_mode_valid_fixed(), which validates a given mode
against a display hardware's mode. Convert simpledrm and use it in a
few other drivers with static modes.
v4:
* remove empty line after opening brace
v2:
* rename 'static' and 'hw' to 'fixed' everywhere
Signed-off-by: Thomas Zimmermann <[email protected]>
Reviewed-by: Sam Ravnborg <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Smatch warns:
drivers/gpu/drm/vkms/vkms_plane.c:110 vkms_plane_atomic_update() warn:
variable dereferenced before check 'fb' (see line 108)
Fix the warning by moving the dereference after the NULL check.
Fixes: 8ba1648567 ("drm: vkms: Refactor the plane composer to accept new formats")
Signed-off-by: Harshit Mogalapalli <[email protected]>
Reviewed-by: Melissa Wen <[email protected]>
Signed-off-by: Melissa Wen <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
It's better to perform the sync at the very last of the suspend
instead of the pipe-disable function, so that we can catch all pending
URBs (if any).
While we're at it, drop the error code from udl_sync_pending_urb()
since we basically ignore it; instead, give a clear error message
indicating a problem.
Acked-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
In the current design, udl_get_urb() may be called asynchronously
during the driver freeing its URL list via udl_free_urb_list().
The problem is that the sync is determined by comparing the urbs.count
and urbs.available fields, while we clear urbs.count field only once
after udl_free_urb_list() finishes, i.e. during udl_free_urb_list(),
the state becomes inconsistent.
For fixing this inconsistency and also for hardening the locking
scheme, this patch does a slight refactoring of the code around
udl_get_urb() and udl_free_urb_list(). Now urbs.count is updated in
the same spinlock at extracting a URB from the list in
udl_free_url_list().
Acked-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
The alignment of damaged area was needed for the original udlfb driver
that tried to trim the superfluous copies between front and backend
buffers and handle data in long int. It's not the case for udl DRM
driver, hence we can omit the whole unneeded alignment, as well as the
dead code.
Acked-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Restore the display mode whne resuming from suspend. Currently, the
display remains dark.
On resume, the CRTC's mode does not change, but the 'active' flag
changes to 'true'. Taking this into account when considering a mode
switch restores the display mode.
The bug is reproducable by using Gnome with udl and observing the
adapter's suspend/resume behavior.
Actually, the whole check added in udl_simple_display_pipe_enable()
about the crtc_state->mode_changed was bogus. We should drop the
whole check and always apply the mode change in this function.
[ tiwai -- Drop the mode_changed check entirely instead, per Daniel's
suggestion ]
Fixes: 997d33c356 ("drm/udl: Inline DPMS code into CRTC enable and disable functions")
Cc: <[email protected]>
Suggested-by: Daniel Vetter <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
While we have multiple test cases, most of them check multiple
conditions, calling the function that is tested multiple times with
different arguments (with comments that indicate test case boundary).
This usually means that it can be easily converted into multiple test
cases.
Passing output:
============================================================
================= drm_format (18 subtests) =================
[PASSED] drm_format_block_width_invalid
[PASSED] drm_format_block_width_one_plane
[PASSED] drm_format_block_width_two_plane
[PASSED] drm_format_block_width_three_plane
[PASSED] drm_format_block_width_tiled
[PASSED] drm_format_block_height_invalid
[PASSED] drm_format_block_height_one_plane
[PASSED] drm_format_block_height_two_plane
[PASSED] drm_format_block_height_three_plane
[PASSED] drm_format_block_height_tiled
[PASSED] drm_format_min_pitch_invalid
[PASSED] drm_format_min_pitch_one_plane_8bpp
[PASSED] drm_format_min_pitch_one_plane_16bpp
[PASSED] drm_format_min_pitch_one_plane_24bpp
[PASSED] drm_format_min_pitch_one_plane_32bpp
[PASSED] drm_format_min_pitch_two_plane
[PASSED] drm_format_min_pitch_three_plane_8bpp
[PASSED] drm_format_min_pitch_tiled
=================== [PASSED] drm_format ====================
============================================================
Testing complete. Ran 18 tests: passed: 18
Signed-off-by: Michał Winiarski <[email protected]>
Reviewed-by: Maíra Canal <[email protected]>
Signed-off-by: Maíra Canal <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
drm_format_info_* functions don't return bool, and the info variable is a
pointer.
Expecting non-NULL info will cause the test to crash if it is NULL in
checks that follow (which dereference it).
Use appropriate KUNIT_EXPECT/KUNIT_ASSERT variants.
Signed-off-by: Michał Winiarski <[email protected]>
Reviewed-by: Maíra Canal <[email protected]>
Signed-off-by: Maíra Canal <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Commit ec491291dc ("drm/sun4i: tv: Merge mode_set into atomic_enable")
has introduced a call to drm_atomic_get_new_crtc_state(), but didn't
include drm_atomic.h.
On CONFIG_OF systems, this is fine because drm_atomic.h is included by
drm_of.h through drm_bridge.h. However, whenever CONFIG_OF isn't set, we
end up with a compilation error.
Fixes: ec491291dc ("drm/sun4i: tv: Merge mode_set into atomic_enable")
Reported-by: kernel test robot <[email protected]>
Reported-by: Ville Syrjälä <[email protected]>
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
The simple display pipeline is a set of helpers that can be used by DRM
drivers to avoid dealing with all the needed components and just define
a few functions to operate a simple display device with one full-screen
scanout buffer feeding a single output.
But it is arguable that this provides the correct level of abstraction
for simple drivers, and recently some have been ported from using these
simple display helpers to use the regular atomic helpers instead.
The rationale for this is that the simple display pipeline helpers don't
hide that much of the DRM complexity, while adding an indirection layer
that conflates the concepts of CRTCs and planes. This makes the helpers
less flexible and harder to be reused among different graphics drivers.
Also, for simple drivers, using the full atomic helpers doesn't require
a lot of additional code. So adding a simple display pipeline layer may
not be worth it.
For these reasons, let's follow that trend and make ssd130x a plain DRM
driver that creates its own primary plane, CRTC, enconder and connector.
Suggested-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Details:
Currently, when trying to suspend and resume with qxl device，
there are some error messages after resuming,
eventually caused to black screen and can't be recovered.
The first error message:
[ 64.668577][ C3] [drm] driver is in bug mode
This error is due to guest qxl driver
will call qxl_reinit_memslots(qdev) during system resume,
but didn't call qxl_io_reset(qdev) before this,
Then will cause the QXL_IO_MEMSLOT_ADD operation to fail on QEMU,
qxl->guest_bug flag will be set，As a result,
the QXL device can't communicate with guest qxl driver through the IO port.
after fix the first error,can success to resume and login to desktop,
but shortly after that will observe the second error message :
[ 353.095343][ T863] qxl 0000:00:02.0: object_init failed for (262144, 0x00000001)
[ 353.096660][ T863] [drm:qxl_gem_object_create [qxl]] *ERROR* Failed to allocate GEM object (260852, 1, 4096, -12)
[ 353.097277][ T863] [drm:qxl_alloc_ioctl [qxl]] *ERROR* qxl_alloc_ioctl: failed to create gem ret=-12
[ 368.197538][ T863] qxl 0000:00:02.0: object_init failed for (3149824, 0x00000001)
[ 368.197541][ T863] [drm:qxl_alloc_bo_reserved [qxl]] *ERROR* failed to allocate VRAM BO
The problem is caused by calling qxl_ring_init_hdr(qdev->release_ring)
in qxl_drm_resume() function.
When do QXL_IO_RESET,QEMU will call init_qxl_ram(),
so params like prod,cons,notify_on_cons and notify_on_prod
will be set to default value.
Ring push/pop actions for release_ring can be performed normally.
But call qxl_ring_init_hdr(qdev->release_ring)
will eventually set notify_on_prod to number of QXL_RELEASE_RING_SIZE,
affect the value of notify in qxl_push_free_res() function always be false,
QEMU will no longer send events of QXL_INTERRUPT_DISPLAY to the
guest qxl driver，so qxl_ring_pop() will never been called anymore，
and can't do dma_fence_signal(),result to ttm_bo_wait_ctx(bo, ctx)
always return EBUSY,fail to call qxl_bo_create().
Test scenario:
1) start virtual machine with qemu command "-device qxl-vga"
2) click suspend botton to enter suspend mode
3) resume and observe the error message in kernel logs,screen will be black
Let's fix this by reset io and remove the qxl_ring_init_hdr calling.
Signed-off-by: Zongmin Zhou<[email protected]>
Suggested-by: Ming Xie<[email protected]>
Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
Signed-off-by: Gerd Hoffmann <[email protected]>
VGA_IS1_RC is the color mode register (VGA_IS1_RM the one for monochrome
mode, note C vs. M at the end). So when using VGA_IS1_RC make sure the
vga device is actually in color mode and set the corresponding bit in the
misc register.
Reproducible when booting VMs in UEFI mode with some edk2 versions (edk2
fix is on the way too). Doesn't happen in BIOS mode because in that
case the vgabios already flips the bit.
Fixes: 250e743915 ("drm/bochs: Add screen blanking support")
Signed-off-by: Gerd Hoffmann <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
Without this, the drm core advertises LINEAR modifier which is
incorrect.
Also userspace virgl does not support modifiers. For example, it causes
chrome on ozone/drm to fail with "Failed to create scanout buffer".
Fixes: 2af104290d ("drm: introduce fb_modifiers_not_supported flag in mode_config")
Suggested-by: Shao-Chuan Lee <[email protected]>
Signed-off-by: Chia-I Wu <[email protected]>
Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
Signed-off-by: Gerd Hoffmann <[email protected]>
The other error labels in sun4i_tv_bind() are named after the task they
perform (err_disable_clk to call clk_disable_unprepare for example).
However, the err_cleanup_connector is named after the calling site
(drm_connector_init failing) and will actually cleanup the encoder. Let's
rename it to err_cleanup_encoder to be consistent.
Reviewed-by: Jernej Skrabec <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Maxime Ripard