mbox series

[v3,0/2] drm: Allow encoder modeset when connectors are changed

Message ID 20241211-abhinavk-modeset-fix-v3-0-0de4bf3e7c32@quicinc.com (mailing list archive)
Headers show
Series drm: Allow encoder modeset when connectors are changed | expand

Message

Jessica Zhang Dec. 11, 2024, 9:18 p.m. UTC
Call encoder mode_set() when connectors are changed. This avoids issues
for cases where the connectors are changed but CRTC mode is not.

---
Changes in v3:
- BUILD_BUG_ON() for encoders and connectors array size check (Dmitry)
- Added more descriptive, file-specific names for helper functions
  (Dmitry)
- Added comment to test documenting what the test does (Dmitry)
- Return drm_connector_helper_get_modes_fixed() directly for encoder
  get_modes() instead of returning 1 (Dmitry)
- Move local variable declarations to top of function (Dmitry)
- Link to v2: https://lore.kernel.org/r/20241209-abhinavk-modeset-fix-v2-0-4d008f6ea8d0@quicinc.com

Changes in v2:

- Added kunit test

---
Abhinav Kumar (1):
      drm: allow encoder mode_set even when connectors change for crtc

Jessica Zhang (1):
      drm/tests: Add test for drm_atomic_helper_commit_modeset_disables()

 drivers/gpu/drm/drm_atomic_helper.c           |   2 +-
 drivers/gpu/drm/tests/Makefile                |   1 +
 drivers/gpu/drm/tests/drm_atomic_state_test.c | 244 ++++++++++++++++++++++++++
 3 files changed, 246 insertions(+), 1 deletion(-)
---
base-commit: 86313a9cd152330c634b25d826a281c6a002eb77
change-id: 20241209-abhinavk-modeset-fix-74864f1de08d

Best regards,

Comments

Maxime Ripard Dec. 16, 2024, 11:06 a.m. UTC | #1
On Wed, Dec 11, 2024 at 01:18:41PM -0800, Jessica Zhang wrote:
> Call encoder mode_set() when connectors are changed. This avoids issues
> for cases where the connectors are changed but CRTC mode is not.

Looks great, thanks a lot for doing the tests :)

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime
Abhinav Kumar Dec. 16, 2024, 6:27 p.m. UTC | #2
On 12/16/2024 3:06 AM, Maxime Ripard wrote:
> On Wed, Dec 11, 2024 at 01:18:41PM -0800, Jessica Zhang wrote:
>> Call encoder mode_set() when connectors are changed. This avoids issues
>> for cases where the connectors are changed but CRTC mode is not.
> 
> Looks great, thanks a lot for doing the tests :)
> 
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> 
> Maxime

Thanks for your feedback.

Can we get an ack to land this through msm tree as part of the series 
which needed it?
Maxime Ripard Dec. 17, 2024, 3:02 p.m. UTC | #3
On Mon, Dec 16, 2024 at 10:27:44AM -0800, Abhinav Kumar wrote:
> 
> 
> On 12/16/2024 3:06 AM, Maxime Ripard wrote:
> > On Wed, Dec 11, 2024 at 01:18:41PM -0800, Jessica Zhang wrote:
> > > Call encoder mode_set() when connectors are changed. This avoids issues
> > > for cases where the connectors are changed but CRTC mode is not.
> > 
> > Looks great, thanks a lot for doing the tests :)
> > 
> > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> > 
> > Maxime
> 
> Thanks for your feedback.
> 
> Can we get an ack to land this through msm tree as part of the series which
> needed it?

If possible, I'd rather merge it through drm-misc. We merge a
significant number of patches affecting the framework there, so a
conflict would be less likely there.

Maxime
Dmitry Baryshkov Dec. 20, 2024, 2:05 a.m. UTC | #4
On Tue, Dec 17, 2024 at 04:02:21PM +0100, Maxime Ripard wrote:
> On Mon, Dec 16, 2024 at 10:27:44AM -0800, Abhinav Kumar wrote:
> > 
> > 
> > On 12/16/2024 3:06 AM, Maxime Ripard wrote:
> > > On Wed, Dec 11, 2024 at 01:18:41PM -0800, Jessica Zhang wrote:
> > > > Call encoder mode_set() when connectors are changed. This avoids issues
> > > > for cases where the connectors are changed but CRTC mode is not.
> > > 
> > > Looks great, thanks a lot for doing the tests :)
> > > 
> > > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> > > 
> > > Maxime
> > 
> > Thanks for your feedback.
> > 
> > Can we get an ack to land this through msm tree as part of the series which
> > needed it?
> 
> If possible, I'd rather merge it through drm-misc. We merge a
> significant number of patches affecting the framework there, so a
> conflict would be less likely there.

I think it should be fine to merge this patchset + core part of the
Jessica's CWB patches ([1]) through drm-misc, then merge drm-misc-next
into msm-next. I'd ask for such a merge anyway, once Vignesh's IGT uprev
[2] lands in drm-misc as I need it to finally land the patchset
converting msm/hdmi to use the HDMI Connector framework.

[1] https://lore.kernel.org/dri-devel/20241216-concurrent-wb-v4-0-fe220297a7f0@quicinc.com/#r
[2] https://lore.kernel.org/dri-devel/20241217160655.2371138-1-vignesh.raman@collabora.com/