[Intel-gfx] [PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch

[Srinivas, Vidya]

Apologies. This patch was pushed wrongly. Not a part of the 16 patch series of NV12.

Regards
Vidya

> -----Original Message-----
> From: Sharma, Shashank
> Sent: Thursday, February 8, 2018 6:08 PM
> To: Srinivas, Vidya <vidya.srinivas at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: maarten.lankhorst at linux.intel.com; Kamath, Sunil
> <sunil.kamath at intel.com>; Shankar, Uma <uma.shankar at intel.com>;
> Konduru, Chandra <chandra.konduru at intel.com>
> Subject: Re: [PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch
> 
> Regards
> 
> Shashank
> 
> 
> On 2/6/2018 4:36 PM, Vidya Srinivas wrote:
> > From: Chandra Konduru <chandra.konduru at intel.com>
> >
> > Display WA #0827:
> > Switching the plane format from NV12 to RGB and leaving system idle
> > results in display underrun and corruption. WA: Set the bit 15 & bit
> > 19 to 1b in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane
> is enabled.
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru at intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas at intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_reg.h      |  3 +++
> >   drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
> >   2 files changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 8f36023..c4af05e 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3822,6 +3822,9 @@ enum {
> >   #define _CLKGATE_DIS_PSL_A		0x46520
> >   #define _CLKGATE_DIS_PSL_B		0x46524
> >   #define _CLKGATE_DIS_PSL_C		0x46528
> > +#define DUPS1_GATING_DIS	(1 << 15)
> > +#define DUPS2_GATING_DIS	(1 << 19)
> > +#define DUPS3_GATING_DIS	(1 << 23)
> Bit definition should be aligned by one extra space (like below), also the bit
> sequence should be high -> low (so 23,19 and then 15)
> >   #define   DPF_GATING_DIS		(1 << 10)
> >   #define   DPF_RAM_GATING_DIS		(1 << 9)
> >   #define   DPFR_GATING_DIS		(1 << 8)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 551c970..94faf3e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5495,6 +5495,20 @@ static void
> glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
> >   	I915_WRITE(CLKGATE_DIS_PSL(pipe), val);
> >   }
> >
> > +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> > +	int pipe, int enable)
> Do we need an int ? or bool enable ? also This line should be aligned to
> opening brace '(' above.
> > +{
> > +	if (pipe == PIPE_A || pipe == PIPE_B) {
> > +		if (enable)
> > +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > +				DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> Alignment, also we are overwriting all other bits here, this should be
> I915_WRITE(CLKGATE_DIS_PSL(pipe), I915_READ(CLKGATE_DIS_PSL(pipe))
> |= (DUPS1_GATING_DIS | DUPS2_GATING_DIS) )
> > +		else
> > +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > +			I915_READ(CLKGATE_DIS_PSL(pipe)) &
> This line should be aligned to the '(' above
> > +			~(DUPS1_GATING_DIS|DUPS2_GATING_DIS));
> > +	}
> > +}
> > +
> >   static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> >   				struct drm_atomic_state *old_state)
> >   {
> > @@ -5599,6 +5613,7 @@ static void haswell_crtc_enable(struct
> intel_crtc_state *pipe_config,
> >   		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
> >   		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
> >   	}
> > +	skl_wa_clkgate(dev_priv, pipe, 1);
> send true from here (instead of 1)
> >   }
> >
> >   static void ironlake_pfit_disable(struct intel_crtc *crtc, bool
> > force) @@ -5709,6 +5724,7 @@ static void haswell_crtc_disable(struct
> intel_crtc_state *old_crtc_state,
> >   		intel_ddi_disable_pipe_clock(intel_crtc->config);
> >
> >   	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
> > +	skl_wa_clkgate(dev_priv, intel_crtc->pipe, 0);
> send false from here (instead of 0)
> >   }
> >
> >   static void i9xx_pfit_enable(struct intel_crtc *crtc)