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

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

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.

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx