Em Sex, 2017-09-01 às 14:31 -0700, Rodrigo Vivi escreveu:
> On Fri, Sep 1, 2017 at 2:04 PM, Paulo Zanoni <paulo.r.zanoni at intel.co
> m> wrote:
> > Em Ter, 2017-08-29 às 16:08 -0700, Rodrigo Vivi escreveu:
> > > Skip compressing 1 segment at the end of the frame,
> > > avoid a pixel count mismatch nuke event when last active
> > > pixel and dummy pixel has same color for Odd Plane
> > > Width / Height.
> > >
> > > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > > drivers/gpu/drm/i915/intel_pm.c | 4 ++++
> > > 2 files changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index e2908ae34004..0072ef79bf34 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -2939,6 +2939,8 @@ enum i915_power_well_id {
> > > #define ILK_DPFC_CHICKEN _MMIO(0x43224)
> > > #define ILK_DPFC_DISABLE_DUMMY0 (1<<8)
> > > #define ILK_DPFC_NUKE_ON_ANY_MODIFICATION (1<<23)
> > > +#define CNL_SKIP_SEG_EN (1<<12)
> > > +#define CNL_SKIP_SEG_COUNT (1<<10)
> >
> > #define CNL_SKIP_SEG_COUNT_MASK (3 << 10)
> > #define CNL_SKIP_SEG_COUNT(x) ((x) << 10)
>
> By the definition we should add the SHIFT then as well?
> Although we are not going to read ever?!
I don't see the point in adding it if we're not using it. Most of the
times where we have the function-like macro we don't need _SHIFT, but
_MASK is needed due to the RMW.
>
> >
> > Also, since you're going to submit v2 anyway, I wouldn't complain
> > if
> > you drive-by fixed the definitions of ILK_DPFC_CHICKEN bits to use
> > spaces as documented in the beginning of the file. If we fix every
> > time
> > we touch a reg, we may at some point be consistent :).
>
> Last patch I tried to fix the style on the same patch Jani nacked it.
> So I wonder if we need a bit patch to fix all the style entirely.
I just gave a suggestion, fixing the spacing is not required in this
patch.
>
> >
> >
> > > #define ILK_FBC_RT_BASE _MMIO(0x2128)
> > > #define ILK_FBC_RT_VALID (1<<0)
> > > #define SNB_FBC_FRONT_BUFFER (1<<1)
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index d5ff0b9f999f..acf793256507 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -8283,6 +8283,10 @@ static void
> > > cannonlake_init_clock_gating(struct drm_i915_private *dev_priv)
> > > I915_WRITE(SLICE_UNIT_LEVEL_CLKGATE,
> > > I915_READ(SLICE_UNIT_LEVEL_CLKGATE) |
> > > SARBUNIT_CLKGATE_DIS);
> > > +
> > > + /* WaFbcSkipSegments:cnl */
> >
> > Since this is also documented in BSpec but it doesn't include the
> > WA
> > name, can you please also document that this is WA #1133 in this
> > comment? I'd do something like:
> >
> > /* Display WA #1133: WaFbcSkipSegments:cnl,glk. */
>
> good idea!
>
> >
> > Please notice that we also need to apply this WA for GLK.
>
> hm... indeed...
> >
> >
> > > + I915_WRITE(ILK_DPFC_CHICKEN, I915_READ(ILK_DPFC_CHICKEN) |
> > > + CNL_SKIP_SEG_EN | CNL_SKIP_SEG_COUNT);
> >
> > Need to clear bit 11 too.
>
> ouch! yeap!
>
> Thanks,
> Rodrigo.
>
> >
> > val = READ();
> > val &= ~CNL_SKIP_SEG_COUNT_MASK;
> > val |= CNL_SKIP_SEG_EN | CNL_SKIP_SEG_COUNT(1);
> >
> > > }
> > >
> > > static void kabylake_init_clock_gating(struct drm_i915_private
> > > *dev_priv)
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>