diff mbox series

[1/5] drm/i915: Disable compression tricks on JSL

Message ID 20240624150538.24102-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Enable CCS+10bpc and CCS+async flips | expand

Commit Message

Ville Syrjälä June 24, 2024, 3:05 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bspec asks us to disable some compression trick on JSL. While the
bspec description is pretty vague it looks like this is some extra
trick for 10bpc+ CCS which presumably the ICL derived display engine
doesn't support.

Note that we aren't currently exposing 10bpc CCS scanout support,
but once that gets added this presumably becomes an issue.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_regs.h     | 1 +
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 +++++++++
 2 files changed, 10 insertions(+)

Comments

Andi Shyti June 27, 2024, 10:33 p.m. UTC | #1
Hi Ville,

On Mon, Jun 24, 2024 at 06:05:34PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Bspec asks us to disable some compression trick on JSL. While the
> bspec description is pretty vague it looks like this is some extra
> trick for 10bpc+ CCS which presumably the ICL derived display engine
> doesn't support.
> 
> Note that we aren't currently exposing 10bpc CCS scanout support,
> but once that gets added this presumably becomes an issue.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h     | 1 +
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 +++++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index e42b3a5d4e63..af53c40e6c21 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -432,6 +432,7 @@
>  #define XEHPG_INSTDONE_GEOM_SVG			MCR_REG(0x666c)
>  
>  #define CACHE_MODE_0_GEN7			_MMIO(0x7000) /* IVB+ */
> +#define   DISABLE_REPACKING_FOR_COMPRESSION	REG_BIT(15) /* jsl+ */

I know that REG_BIT() is the correct one, but for conformity I
would still use (1 << 15), otherwise we would have a mess of
styles.

Besides, you are breaking the order here.

>  #define   RC_OP_FLUSH_ENABLE			(1 << 0)
>  #define   HIZ_RAW_STALL_OPT_DISABLE		(1 << 2)
>  #define CACHE_MODE_1				_MMIO(0x7004) /* IVB+ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 09a287c1aedd..a424b442493f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -2286,6 +2286,15 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>  			     GEN8_RC_SEMA_IDLE_MSG_DISABLE);
>  	}
>  
> +	if (IS_JASPERLAKE(i915) || IS_ELKHARTLAKE(i915)) {
> +		/*
> +		 * "Disable Repacking for Compression (masked R/W access)
> +		 *  before rendering compressed surfaces for display."
> +		 */
> +		wa_masked_en(wal, CACHE_MODE_0_GEN7,
> +			     DISABLE_REPACKING_FOR_COMPRESSION);

It is vague, indeed, but the description says that repacking
provides a boost in performance and "improvement in Lossless
Compression Algo" (whatever that means, I'm sailing in unknown
waters here :-) )

How did you get here?

Andi

> +	}
> +
>  	if (GRAPHICS_VER(i915) == 11) {
>  		/* This is not an Wa. Enable for better image quality */
>  		wa_masked_en(wal,
> -- 
> 2.44.2
Ville Syrjälä June 28, 2024, 10:57 a.m. UTC | #2
On Fri, Jun 28, 2024 at 12:33:12AM +0200, Andi Shyti wrote:
> Hi Ville,
> 
> On Mon, Jun 24, 2024 at 06:05:34PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Bspec asks us to disable some compression trick on JSL. While the
> > bspec description is pretty vague it looks like this is some extra
> > trick for 10bpc+ CCS which presumably the ICL derived display engine
> > doesn't support.
> > 
> > Note that we aren't currently exposing 10bpc CCS scanout support,
> > but once that gets added this presumably becomes an issue.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h     | 1 +
> >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 9 +++++++++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index e42b3a5d4e63..af53c40e6c21 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -432,6 +432,7 @@
> >  #define XEHPG_INSTDONE_GEOM_SVG			MCR_REG(0x666c)
> >  
> >  #define CACHE_MODE_0_GEN7			_MMIO(0x7000) /* IVB+ */
> > +#define   DISABLE_REPACKING_FOR_COMPRESSION	REG_BIT(15) /* jsl+ */
> 
> I know that REG_BIT() is the correct one, but for conformity I
> would still use (1 << 15), otherwise we would have a mess of
> styles.

The file is full of both. I guess I could throw in a patch to
finish the conversion...

> 
> Besides, you are breaking the order here.

The order here is wrong for whatever reason. I suppose I
can cook up a patch to fix that too.

> 
> >  #define   RC_OP_FLUSH_ENABLE			(1 << 0)
> >  #define   HIZ_RAW_STALL_OPT_DISABLE		(1 << 2)
> >  #define CACHE_MODE_1				_MMIO(0x7004) /* IVB+ */
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index 09a287c1aedd..a424b442493f 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -2286,6 +2286,15 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> >  			     GEN8_RC_SEMA_IDLE_MSG_DISABLE);
> >  	}
> >  
> > +	if (IS_JASPERLAKE(i915) || IS_ELKHARTLAKE(i915)) {
> > +		/*
> > +		 * "Disable Repacking for Compression (masked R/W access)
> > +		 *  before rendering compressed surfaces for display."
> > +		 */
> > +		wa_masked_en(wal, CACHE_MODE_0_GEN7,
> > +			     DISABLE_REPACKING_FOR_COMPRESSION);
> 
> It is vague, indeed, but the description says that repacking
> provides a boost in performance and "improvement in Lossless
> Compression Algo" (whatever that means, I'm sailing in unknown
> waters here :-) )
> 
> How did you get here?

Bspec:18437 and then I read through the linked HSDs.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index e42b3a5d4e63..af53c40e6c21 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -432,6 +432,7 @@ 
 #define XEHPG_INSTDONE_GEOM_SVG			MCR_REG(0x666c)
 
 #define CACHE_MODE_0_GEN7			_MMIO(0x7000) /* IVB+ */
+#define   DISABLE_REPACKING_FOR_COMPRESSION	REG_BIT(15) /* jsl+ */
 #define   RC_OP_FLUSH_ENABLE			(1 << 0)
 #define   HIZ_RAW_STALL_OPT_DISABLE		(1 << 2)
 #define CACHE_MODE_1				_MMIO(0x7004) /* IVB+ */
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 09a287c1aedd..a424b442493f 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2286,6 +2286,15 @@  rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			     GEN8_RC_SEMA_IDLE_MSG_DISABLE);
 	}
 
+	if (IS_JASPERLAKE(i915) || IS_ELKHARTLAKE(i915)) {
+		/*
+		 * "Disable Repacking for Compression (masked R/W access)
+		 *  before rendering compressed surfaces for display."
+		 */
+		wa_masked_en(wal, CACHE_MODE_0_GEN7,
+			     DISABLE_REPACKING_FOR_COMPRESSION);
+	}
+
 	if (GRAPHICS_VER(i915) == 11) {
 		/* This is not an Wa. Enable for better image quality */
 		wa_masked_en(wal,