diff mbox series

[v2] drm/atomic: Add the crtc to affected crtc only if uapi.enable = true

Message ID 20211004113629.23715-1-manasi.d.navare@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/atomic: Add the crtc to affected crtc only if uapi.enable = true | expand

Commit Message

Navare, Manasi Oct. 4, 2021, 11:36 a.m. UTC
In case of a modeset where a mode gets split across mutiple CRTCs
in the driver specific implementation (bigjoiner in i915) we wrongly count
the affected CRTCs based on the drm_crtc_mask and indicate the stolen CRTC as
an affected CRTC in atomic_check_only().
This triggers a warning since affected CRTCs doent match requested CRTC.

To fix this in such bigjoiner configurations, we should only
increment affected crtcs if that CRTC is enabled in UAPI not
if it is just used internally in the driver to split the mode.

There is no way we can adjust requested_crtc calculation as suggested
in review comments because the crtc gets stolen only after the atomic_check call.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Simon Ser <contact@emersion.fr>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Ville Syrjala Oct. 4, 2021, 11:37 a.m. UTC | #1
On Mon, Oct 04, 2021 at 04:36:29AM -0700, Manasi Navare wrote:
> In case of a modeset where a mode gets split across mutiple CRTCs
> in the driver specific implementation (bigjoiner in i915) we wrongly count
> the affected CRTCs based on the drm_crtc_mask and indicate the stolen CRTC as
> an affected CRTC in atomic_check_only().
> This triggers a warning since affected CRTCs doent match requested CRTC.
> 
> To fix this in such bigjoiner configurations, we should only
> increment affected crtcs if that CRTC is enabled in UAPI not
> if it is just used internally in the driver to split the mode.
> 
> There is no way we can adjust requested_crtc calculation as suggested
> in review comments because the crtc gets stolen only after the atomic_check call.

The uapi crtc_state->enable value does not change due to bigjoiner
stealing the crtc. So I don't understand what you're trying to say
here.

The only internal thing that could alter the set of enabled crtcs
on the uapi level is update_connector_routing(true), but that is
always called before drm_atomic_check_only().

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Simon Ser <contact@emersion.fr>
> Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index ff1416cd609a..44e7ebf43a2a 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1360,8 +1360,10 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
>  		}
>  	}
>  
> -	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
> -		affected_crtc |= drm_crtc_mask(crtc);
> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (new_crtc_state->enable)
> +			affected_crtc |= drm_crtc_mask(crtc);
> +	}
>  
>  	/*
>  	 * For commits that allow modesets drivers can add other CRTCs to the
> -- 
> 2.19.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index ff1416cd609a..44e7ebf43a2a 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1360,8 +1360,10 @@  int drm_atomic_check_only(struct drm_atomic_state *state)
 		}
 	}
 
-	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
-		affected_crtc |= drm_crtc_mask(crtc);
+	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+		if (new_crtc_state->enable)
+			affected_crtc |= drm_crtc_mask(crtc);
+	}
 
 	/*
 	 * For commits that allow modesets drivers can add other CRTCs to the