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

[Mahesh Kumar]

Hi


On Friday 21 July 2017 06:20 PM, Imre Deak wrote:
> On Fri, Jul 21, 2017 at 05:41:13PM +0530, Mahesh Kumar wrote:
>> 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..
> They are removed in the next patch. I didn't want to remove them in one
> go, since for stable I wanted to keep the changes minimal.
OK, l'm fine with it.
Reviewed-by: Mahesh Kumar <mahesh1.kumar at intel.com>
>
>> Should not we reading actual HW state of scaler here instead of just
>> clearing it.
> We will read the scaler state later after determining that the crtc is
> on. If it's off the scaler is also off matching the default state we set
> here.
>
>> -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