[Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout

[Mahesh Kumar]

Hi,


On Thursday 20 July 2017 08:19 PM, Sharma, Shashank wrote:
> Acked-by: Shashank Sharma <shashank.sharma at intel.com>
>
> Regards
> Shashank
> On 7/20/2017 4:20 AM, Imre Deak wrote:
>> The scaler allocation code depends on a non-zero default value for the
>> crtc scaler_id, so make sure we initialize the scaler state accordingly
>> even if the crtc is off. This fixes at least an initial YUV420 modeset
>> (added in a follow-up patchset by Shashank) when booting with the screen
>> off: after the initial HW readout and modeset which enables the scaler a
>> subsequent modeset will disable the scaler which isn't properly
>> allocated. This results in a funky HW state where the pipe scaler HW
>> registers can't be modified and the normally black screen is grey and
>> shifted to the right or jitters.
>>
>> The problem was revealed by Shashank's YUV420 patchset and first
>> reported by Ville.
>>
>> Cc: Shashank Sharma <shashank.sharma at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Chandra Konduru <chandra.konduru at intel.com>
>> Cc: Matt Roper <matthew.d.roper at intel.com>
>> Cc: <stable at vger.kernel.org> # 4.11.x
>> Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared 
>> scalers")
>> Signed-off-by: Imre Deak <imre.deak at intel.com>
>>
>> ---
>>
>> [ Older stable versions need backporting, so that's for a follow-up ]
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 7774f3465fbc..8a38e64b1931 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct 
>> intel_crtc *crtc,
>>       u64 power_domain_mask;
>>       bool active;
>>   +    if (INTEL_GEN(dev_priv) >= 9) {
>> +        intel_crtc_init_scalers(crtc, pipe_config);
>> +
>> +        pipe_config->scaler_state.scaler_id = -1;
>> +        pipe_config->scaler_state.scaler_users &= ~(1 << 
>> SKL_CRTC_INDEX);
We are already setting scaler_id to -1 during init_scalers & doing 
memset over intel_crtc_state in intel_modeset_readout_hw_state, So IMO 
above 2 lines are unnecessary..
Should not we reading actual HW state of scaler here instead of just 
clearing it.

-Mahesh
>> +    }
>> +
>>       power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>>       if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>>           return false;
>> @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct 
>> intel_crtc *crtc,
>>       pipe_config->gamma_mode =
>>           I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>>   -    if (INTEL_GEN(dev_priv) >= 9) {
>> -        intel_crtc_init_scalers(crtc, pipe_config);
>> -
>> -        pipe_config->scaler_state.scaler_id = -1;
>> -        pipe_config->scaler_state.scaler_users &= ~(1 << 
>> SKL_CRTC_INDEX);
>> -    }
>> -
>>       power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
>>       if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>>           power_domain_mask |= BIT_ULL(power_domain);
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx