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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > Cc: Chandra Konduru <chandra.konduru@xxxxxxxxx>
> > > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> > > Cc: <stable@xxxxxxxxxxxxxxx> # 4.11.x
> > > Reported-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared
> > > scalers")
> > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > >
> > > ---
> > >
> > > [ 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.
> 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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>