Re: [PATCH v2 14/14] drm/i915/skl: Do not allow scaling when crtc is disabled.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Nov 03, 2015 at 11:06:53AM +0100, Maarten Lankhorst wrote:
> Op 03-11-15 om 10:09 schreef Ville Syrjälä:
> > On Tue, Nov 03, 2015 at 08:31:53AM +0100, Maarten Lankhorst wrote:
> >> This fixes a warning when the crtc is turned off. In that case fb
> >> will be NULL, and crtc_clock will be 0. Because the crtc is no longer
> >> active this is not a bug, and shouldn't trigger the WARN_ON.
> > Mm. We want to do scaling checks and whatnot during DPMS. So this should
> > check .enabled, no?
> Not sure what the right decision would be here..
> 
>      * skl max scale is lower of:
>      *    close to 3 but not 3, -1 is for that purpose
>      *            or
>      *    cdclk/crtc_clock
> 
> So when multiple pipes are enabled potentially 3x scaling is allowed, but if you dpms them all off
> cdclk might get set to 0. This means a previous valid amount of scaling might suddenly become invalid.
> 
> Maybe the fix is keeping 2 cdclk's, one for scaling and one for setting.

I think we should keep around the min required cdclk in the crtc state.
And we recalculate that one every time anything changes. Then we can
just compare it against the current cdclk after plane/crtc states have
otherwise been computed to see if we need to change the current cdclk.

> 
> This probably needs to be addressed in patch 3/14 then, so that one needs more love.
> 
> I will resend patch 3, 4, 5 and 14 as a separate series. The other patches from this series will still work with some easily fixed rejects.
> >> Also remove handling a null crtc_state, with all transitional helpers
> >> gone this can no longer happen.
> > What about the !intel_crtc check, how is that supposed to happen?
> When fb == NULL. :)
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 304e1028c9a4..7e2caeef9a11 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -13671,7 +13671,7 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
> >>  	struct drm_i915_private *dev_priv;
> >>  	int crtc_clock, cdclk;
> >>  
> >> -	if (!intel_crtc || !crtc_state)
> >> +	if (!intel_crtc || !crtc_state->base.active)
> >>  		return DRM_PLANE_HELPER_NO_SCALING;
> >>  
> >>  	dev = intel_crtc->base.dev;
> >> -- 
> >> 2.1.0
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx