Re: [PATCH] drm/i915: Update crtc scaler settings when update_pipe is set

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 18-12-18 12:07, Maarten Lankhorst wrote:
> Op 17-12-2018 om 15:19 schreef Hans de Goede:
>> When the pipe_config's update_pipe flag is set we may need to update the
>> panel fitting settings. On GEN9+ this means we need to update the crtc's
>> scaler settings.
>>
>> This fixes the following WARN_ON, during i915 loading on an Asrock
>> B150M Pro4S/D3 board with an i5-6500 CPU / graphics:
>>
>> [drm:pipe_config_err [i915]] *ERROR* mismatch in pch_pfit.enabled
>>   (expected no, found yes)
>> pipe state doesn't match!
>> WARNING: CPU: 3 PID: 305 at drivers/gpu/drm/i915/intel_display.c:12084
>>
>> With line 12084 being the I915_STATE_WARN call inside the
>> "if (!intel_pipe_config_compare())" block in verify_crtc_state().
>>
>> On this board with 2 1920x1080 monitors connected over HDMI the GOP
>> initializes both monitors at 1920x1080 and despite no scaling being
>> necessary configures a scaler for one of them.
>>
>> When booting with fastboot=1 on the initial modeset needs_modeset will
>> be false while update_pipe is true. Since we were not calling
>> skl_update_scaler_crtc() in this case we would leave the scaler enabled
>> causing this error.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.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 62df34d30b1f..df32626e0810 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -10919,7 +10919,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>>   	}
>>   
>>   	if (INTEL_GEN(dev_priv) >= 9) {
>> -		if (mode_changed)
>> +		if (mode_changed || pipe_config->update_pipe)
>>   			ret = skl_update_scaler_crtc(pipe_config);
>>   
>>   		if (!ret)
> 
> Hey,
> 
> Pushed.

Thank you.

> You might also be interested in this patch / series, would be nice to have a review on it.
> 
> https://patchwork.freedesktop.org/patch/268410/

I don't feel comfortable reviewing the backlight patch, and the last patch in the
series, which unconditionally enables fastset seems a bit controversial.

IMHO it would be better to start with a patch which just enables this for GEN9+
(and valleyview and cherryview (*)).

It would be interesting to see what the CI thinks of just patches 1-4, with the last
one dropped.

The 2 other patches look good to me, I will reply to them with my Reviewed-by.

Regards,

Hans


*) As a side / hobby project I've been doing hw-enablement for Bay and Cherry Trail, as
such I've a ton of devices and I always run with fastboot=1 so I'm confident that it is
fine there,\ actually I've 2 devices where the LCD panel will not lightup with fastboot=0
(I've debugged this but failed to come up with anything useful).
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel