Re: [RFC PATCH] drm: allow encoder mode_set even when connectors change for crtc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Maxime

On 9/9/2024 6:37 AM, Maxime Ripard wrote:
Hi,

On Thu, Sep 05, 2024 at 03:11:24PM GMT, Abhinav Kumar wrote:
In certain use-cases, a CRTC could switch between two encoders
and because the mode being programmed on the CRTC remains
the same during this switch, the CRTC's mode_changed remains false.
In such cases, the encoder's mode_set also gets skipped.

Skipping mode_set on the encoder for such cases could cause an issue
because even though the same CRTC mode was being used, the encoder
type could have changed like the CRTC could have switched from a
real time encoder to a writeback encoder OR vice-versa.

Allow encoder's mode_set to happen even when connectors changed on a
CRTC and not just when the mode changed.

Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>

The patch and rationale looks sane to me, but we should really add kunit
tests for that scenarii.


Thanks for the review.

We have a IGT for recreating this scenario and thats how this issue was captured

kms_writeback --run-subtest writeback-check-output -c <primary display mode>

We had added an option ( 'c' - custom mode) a couple of yrs ago to allow writeback to be tested using any mode the user passes in (https://lore.kernel.org/r/all/YuJhGkkxah9U6FGx@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/)

If we pass in the same resolution as the primary RT display, this scenario always happens as the CRTC switches between RT encoder and WB encoder. Hope that addresses some of the concern.

Regarding KUnit tests, I have a couple of questions:

1) This is more of a run-time scenario where CRTC switch happens, does this qualify for a KUnit or perhaps I am missing something.

2) Is there any existing KUnit test file under drm/tests for validating drm_atomic_helper_commit_modeset_disables() / drm_atomic_helper_commit_modeset_enables() path because this will fall under that bucket. I didnt find any matching case where we can extend this.

Thanks

Abhinav

Maxime




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux