[Intel-gfx] [PATCH] drm/i915/cnl: WaFbcSkipSegments

[Rodrigo Vivi]

On Fri, Sep 1, 2017 at 2:04 PM, Paulo Zanoni <paulo.r.zanoni at intel.com> 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?!

>
> 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.

>
>
>>  #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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br