[v3,20/21] drm/i915/display: perform transient flush

Message ID	20240415081423.495834-21-balasubramani.vivekanandan@intel.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <intel-gfx-bounces@lists.freedesktop.org> From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com> To: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Cc: Lucas De Marchi <lucas.demarchi@intel.com>, Matt Roper <matthew.d.roper@intel.com>, Matthew Auld <matthew.auld@intel.com>, Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>, Nirmoy Das <nirmoy.das@intel.com> Subject: [PATCH v3 20/21] drm/i915/display: perform transient flush Date: Mon, 15 Apr 2024 13:44:22 +0530 Message-Id: <20240415081423.495834-21-balasubramani.vivekanandan@intel.com> In-Reply-To: <20240415081423.495834-1-balasubramani.vivekanandan@intel.com> References: <20240415081423.495834-1-balasubramani.vivekanandan@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: list Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
Series	Enable display support for Battlemage \| expand [v3,00/21] Enable display support for Battlemage [v3,01/21] drm/xe/display: Lane reversal requires writes to both context lanes [v3,02/21] drm/i915/display: Enable RM timeout detection [v3,03/21] drm/i915/bmg: Define IS_BATTLEMAGE macro [v3,04/21] drm/i915/xe2hpd: Skip CCS modifiers [v3,05/21] drm/i915/xe2hpd: Initial cdclk table [v3,06/21] drm/i915/bmg: Extend DG2 tc check to future [v3,07/21] drm/i915/xe2hpd: Properly disable power in port A [v3,08/21] drm/i915/xe2hpd: Add new C20 PHY SRAM address [v3,09/21] drm/i915/xe2hpd: Add support for eDP PLL configuration [v3,10/21] drm/i915/xe2hpd: update pll values in sync with Bspec [v3,11/21] drm/i915/xe2hpd: Add display info [v3,12/21] drm/i915/xe2hpd: Configure CHICKEN_MISC_2 before enabling planes [v3,13/21] drm/i915/xe2hpd: Add max memory bandwidth algorithm [v3,14/21] drm/i915/xe2hpd: Do not program MBUS_DBOX BW credits [v3,15/21] drm/i915/bmg: BMG should re-use MTL's south display logic [v3,16/21] Revert "drm/i915/dgfx: DGFX uses direct VBT pin mapping" [v3,17/21] drm/i915/xe2hpd: Set maximum DP rate to UHBR13.5 [v3,18/21] drm/xe/gt_print: add xe_gt_err_once() [v3,19/21] drm/xe/device: implement transient flush [v3,20/21] drm/i915/display: perform transient flush [v3,21/21] drm/xe/bmg: Enable the display support

Message ID

20240415081423.495834-21-balasubramani.vivekanandan@intel.com (mailing list archive)

State

New, archived

Headers

From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
To: intel-xe@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org
Cc: Lucas De Marchi <lucas.demarchi@intel.com>,
 Matt Roper <matthew.d.roper@intel.com>,
 Matthew Auld <matthew.auld@intel.com>,
 Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>,
 Nirmoy Das <nirmoy.das@intel.com>
Subject: [PATCH v3 20/21] drm/i915/display: perform transient flush
Date: Mon, 15 Apr 2024 13:44:22 +0530
Message-Id: <20240415081423.495834-21-balasubramani.vivekanandan@intel.com>
In-Reply-To: <20240415081423.495834-1-balasubramani.vivekanandan@intel.com>
References: <20240415081423.495834-1-balasubramani.vivekanandan@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: list
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Series

Enable display support for Battlemage | expand

Commit Message

Vivekanandan, Balasubramani April 15, 2024, 8:14 a.m. UTC

From: Matthew Auld <matthew.auld@intel.com>

Perform manual transient cache flush prior to flip and at the end of
frontbuffer_flush. This is needed to ensure display engine doesn't see
garbage if the surface is L3:XD dirty.

Testcase: igt@xe-pat@display-vs-wb-transient
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
Acked-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  3 +++
 .../gpu/drm/i915/display/intel_frontbuffer.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_tdf.h      | 25 +++++++++++++++++++
 drivers/gpu/drm/xe/Makefile                   |  3 ++-
 drivers/gpu/drm/xe/display/xe_tdf.c           | 13 ++++++++++
 5 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_tdf.h
 create mode 100644 drivers/gpu/drm/xe/display/xe_tdf.c

Comments

Matt Roper April 15, 2024, 5:07 p.m. UTC | #1

On Mon, Apr 15, 2024 at 01:44:22PM +0530, Balasubramani Vivekanandan wrote:
> From: Matthew Auld <matthew.auld@intel.com>
> 
> Perform manual transient cache flush prior to flip and at the end of
> frontbuffer_flush. This is needed to ensure display engine doesn't see
> garbage if the surface is L3:XD dirty.
> 
> Testcase: igt@xe-pat@display-vs-wb-transient

Has the IGT patch for this been sent yet?  If not, we should probably
make sure that happens soon, and then use the CI Test-with: thing if
there winds up being another revision of this series so that this will
be included in the CI results.

Anyway, the changes here look good to me,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> Acked-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  3 +++
>  .../gpu/drm/i915/display/intel_frontbuffer.c  |  2 ++
>  drivers/gpu/drm/i915/display/intel_tdf.h      | 25 +++++++++++++++++++
>  drivers/gpu/drm/xe/Makefile                   |  3 ++-
>  drivers/gpu/drm/xe/display/xe_tdf.c           | 13 ++++++++++
>  5 files changed, 45 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_tdf.h
>  create mode 100644 drivers/gpu/drm/xe/display/xe_tdf.c
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 67697d9a559c..4fc46edcb4ad 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -110,6 +110,7 @@
>  #include "intel_sdvo.h"
>  #include "intel_snps_phy.h"
>  #include "intel_tc.h"
> +#include "intel_tdf.h"
>  #include "intel_tv.h"
>  #include "intel_vblank.h"
>  #include "intel_vdsc.h"
> @@ -7242,6 +7243,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>  
>  	intel_atomic_commit_fence_wait(state);
>  
> +	intel_td_flush(dev_priv);
> +
>  	drm_atomic_helper_wait_for_dependencies(&state->base);
>  	drm_dp_mst_atomic_wait_for_dependencies(&state->base);
>  	intel_atomic_global_state_wait_for_dependencies(state);
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> index 2ea37c0414a9..4923c340a0b6 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> @@ -65,6 +65,7 @@
>  #include "intel_fbc.h"
>  #include "intel_frontbuffer.h"
>  #include "intel_psr.h"
> +#include "intel_tdf.h"
>  
>  /**
>   * frontbuffer_flush - flush frontbuffer
> @@ -93,6 +94,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
>  	trace_intel_frontbuffer_flush(i915, frontbuffer_bits, origin);
>  
>  	might_sleep();
> +	intel_td_flush(i915);
>  	intel_drrs_flush(i915, frontbuffer_bits);
>  	intel_psr_flush(i915, frontbuffer_bits, origin);
>  	intel_fbc_flush(i915, frontbuffer_bits, origin);
> diff --git a/drivers/gpu/drm/i915/display/intel_tdf.h b/drivers/gpu/drm/i915/display/intel_tdf.h
> new file mode 100644
> index 000000000000..353cde21f6c2
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_tdf.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#ifndef __INTEL_TDF_H__
> +#define __INTEL_TDF_H__
> +
> +/*
> + * TDF (Transient-Data-Flush) is needed for Xe2+ where special L3:XD caching can
> + * be enabled through various PAT index modes. Idea is to use this caching mode
> + * when for example rendering onto the display surface, with the promise that
> + * KMD will ensure transient cache entries are always flushed by the time we do
> + * the display flip, since display engine is never coherent with CPU/GPU caches.
> + */
> +
> +struct drm_i915_private;
> +
> +#ifdef I915
> +static inline void intel_td_flush(struct drm_i915_private *i915) {}
> +#else
> +void intel_td_flush(struct drm_i915_private *i915);
> +#endif
> +
> +#endif
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 6015c9e41f24..97a8674cdd76 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -198,7 +198,8 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
>  	display/xe_dsb_buffer.o \
>  	display/xe_fb_pin.o \
>  	display/xe_hdcp_gsc.o \
> -	display/xe_plane_initial.o
> +	display/xe_plane_initial.o \
> +	display/xe_tdf.o
>  
>  # SOC code shared with i915
>  xe-$(CONFIG_DRM_XE_DISPLAY) += \
> diff --git a/drivers/gpu/drm/xe/display/xe_tdf.c b/drivers/gpu/drm/xe/display/xe_tdf.c
> new file mode 100644
> index 000000000000..2c0d4e144e09
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/display/xe_tdf.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +#include "xe_device.h"
> +#include "intel_display_types.h"
> +#include "intel_tdf.h"
> +
> +void intel_td_flush(struct drm_i915_private *i915)
> +{
> +	xe_device_td_flush(i915);
> +}
> -- 
> 2.25.1
>

Matt Roper April 15, 2024, 6:14 p.m. UTC | #2

On Mon, Apr 15, 2024 at 10:07:32AM -0700, Matt Roper wrote:
> On Mon, Apr 15, 2024 at 01:44:22PM +0530, Balasubramani Vivekanandan wrote:
> > From: Matthew Auld <matthew.auld@intel.com>
> > 
> > Perform manual transient cache flush prior to flip and at the end of
> > frontbuffer_flush. This is needed to ensure display engine doesn't see
> > garbage if the surface is L3:XD dirty.
> > 
> > Testcase: igt@xe-pat@display-vs-wb-transient
> 
> Has the IGT patch for this been sent yet?  If not, we should probably
> make sure that happens soon, and then use the CI Test-with: thing if
> there winds up being another revision of this series so that this will
> be included in the CI results.

Oh, it looks like this test already landed back in early March; I just
didn't look back far enough in the git history originally.  You can
ignore this comment.


Matt

> 
> Anyway, the changes here look good to me,
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> 
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > Acked-by: Nirmoy Das <nirmoy.das@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |  3 +++
> >  .../gpu/drm/i915/display/intel_frontbuffer.c  |  2 ++
> >  drivers/gpu/drm/i915/display/intel_tdf.h      | 25 +++++++++++++++++++
> >  drivers/gpu/drm/xe/Makefile                   |  3 ++-
> >  drivers/gpu/drm/xe/display/xe_tdf.c           | 13 ++++++++++
> >  5 files changed, 45 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/i915/display/intel_tdf.h
> >  create mode 100644 drivers/gpu/drm/xe/display/xe_tdf.c
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 67697d9a559c..4fc46edcb4ad 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -110,6 +110,7 @@
> >  #include "intel_sdvo.h"
> >  #include "intel_snps_phy.h"
> >  #include "intel_tc.h"
> > +#include "intel_tdf.h"
> >  #include "intel_tv.h"
> >  #include "intel_vblank.h"
> >  #include "intel_vdsc.h"
> > @@ -7242,6 +7243,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> >  
> >  	intel_atomic_commit_fence_wait(state);
> >  
> > +	intel_td_flush(dev_priv);
> > +
> >  	drm_atomic_helper_wait_for_dependencies(&state->base);
> >  	drm_dp_mst_atomic_wait_for_dependencies(&state->base);
> >  	intel_atomic_global_state_wait_for_dependencies(state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > index 2ea37c0414a9..4923c340a0b6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > @@ -65,6 +65,7 @@
> >  #include "intel_fbc.h"
> >  #include "intel_frontbuffer.h"
> >  #include "intel_psr.h"
> > +#include "intel_tdf.h"
> >  
> >  /**
> >   * frontbuffer_flush - flush frontbuffer
> > @@ -93,6 +94,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
> >  	trace_intel_frontbuffer_flush(i915, frontbuffer_bits, origin);
> >  
> >  	might_sleep();
> > +	intel_td_flush(i915);
> >  	intel_drrs_flush(i915, frontbuffer_bits);
> >  	intel_psr_flush(i915, frontbuffer_bits, origin);
> >  	intel_fbc_flush(i915, frontbuffer_bits, origin);
> > diff --git a/drivers/gpu/drm/i915/display/intel_tdf.h b/drivers/gpu/drm/i915/display/intel_tdf.h
> > new file mode 100644
> > index 000000000000..353cde21f6c2
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_tdf.h
> > @@ -0,0 +1,25 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2024 Intel Corporation
> > + */
> > +
> > +#ifndef __INTEL_TDF_H__
> > +#define __INTEL_TDF_H__
> > +
> > +/*
> > + * TDF (Transient-Data-Flush) is needed for Xe2+ where special L3:XD caching can
> > + * be enabled through various PAT index modes. Idea is to use this caching mode
> > + * when for example rendering onto the display surface, with the promise that
> > + * KMD will ensure transient cache entries are always flushed by the time we do
> > + * the display flip, since display engine is never coherent with CPU/GPU caches.
> > + */
> > +
> > +struct drm_i915_private;
> > +
> > +#ifdef I915
> > +static inline void intel_td_flush(struct drm_i915_private *i915) {}
> > +#else
> > +void intel_td_flush(struct drm_i915_private *i915);
> > +#endif
> > +
> > +#endif
> > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> > index 6015c9e41f24..97a8674cdd76 100644
> > --- a/drivers/gpu/drm/xe/Makefile
> > +++ b/drivers/gpu/drm/xe/Makefile
> > @@ -198,7 +198,8 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
> >  	display/xe_dsb_buffer.o \
> >  	display/xe_fb_pin.o \
> >  	display/xe_hdcp_gsc.o \
> > -	display/xe_plane_initial.o
> > +	display/xe_plane_initial.o \
> > +	display/xe_tdf.o
> >  
> >  # SOC code shared with i915
> >  xe-$(CONFIG_DRM_XE_DISPLAY) += \
> > diff --git a/drivers/gpu/drm/xe/display/xe_tdf.c b/drivers/gpu/drm/xe/display/xe_tdf.c
> > new file mode 100644
> > index 000000000000..2c0d4e144e09
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/display/xe_tdf.c
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2024 Intel Corporation
> > + */
> > +
> > +#include "xe_device.h"
> > +#include "intel_display_types.h"
> > +#include "intel_tdf.h"
> > +
> > +void intel_td_flush(struct drm_i915_private *i915)
> > +{
> > +	xe_device_td_flush(i915);
> > +}
> > -- 
> > 2.25.1
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation

Nirmoy Das April 15, 2024, 8:55 p.m. UTC | #3

Hi Matt,

On 4/15/2024 7:07 PM, Matt Roper wrote:
> On Mon, Apr 15, 2024 at 01:44:22PM +0530, Balasubramani Vivekanandan wrote:
>> From: Matthew Auld <matthew.auld@intel.com>
>>
>> Perform manual transient cache flush prior to flip and at the end of
>> frontbuffer_flush. This is needed to ensure display engine doesn't see
>> garbage if the surface is L3:XD dirty.
>>
>> Testcase: igt@xe-pat@display-vs-wb-transient
> Has the IGT patch for this been sent yet?

Yes, the test seems to be available 
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/intel/xe_pat.c#L728 



Regards,

Nirmoy

>    If not, we should probably
> make sure that happens soon, and then use the CI Test-with: thing if
> there winds up being another revision of this series so that this will
> be included in the CI results.
>
> Anyway, the changes here look good to me,
>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>> Acked-by: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c  |  3 +++
>>   .../gpu/drm/i915/display/intel_frontbuffer.c  |  2 ++
>>   drivers/gpu/drm/i915/display/intel_tdf.h      | 25 +++++++++++++++++++
>>   drivers/gpu/drm/xe/Makefile                   |  3 ++-
>>   drivers/gpu/drm/xe/display/xe_tdf.c           | 13 ++++++++++
>>   5 files changed, 45 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/i915/display/intel_tdf.h
>>   create mode 100644 drivers/gpu/drm/xe/display/xe_tdf.c
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 67697d9a559c..4fc46edcb4ad 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -110,6 +110,7 @@
>>   #include "intel_sdvo.h"
>>   #include "intel_snps_phy.h"
>>   #include "intel_tc.h"
>> +#include "intel_tdf.h"
>>   #include "intel_tv.h"
>>   #include "intel_vblank.h"
>>   #include "intel_vdsc.h"
>> @@ -7242,6 +7243,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>>   
>>   	intel_atomic_commit_fence_wait(state);
>>   
>> +	intel_td_flush(dev_priv);
>> +
>>   	drm_atomic_helper_wait_for_dependencies(&state->base);
>>   	drm_dp_mst_atomic_wait_for_dependencies(&state->base);
>>   	intel_atomic_global_state_wait_for_dependencies(state);
>> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
>> index 2ea37c0414a9..4923c340a0b6 100644
>> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
>> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
>> @@ -65,6 +65,7 @@
>>   #include "intel_fbc.h"
>>   #include "intel_frontbuffer.h"
>>   #include "intel_psr.h"
>> +#include "intel_tdf.h"
>>   
>>   /**
>>    * frontbuffer_flush - flush frontbuffer
>> @@ -93,6 +94,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
>>   	trace_intel_frontbuffer_flush(i915, frontbuffer_bits, origin);
>>   
>>   	might_sleep();
>> +	intel_td_flush(i915);
>>   	intel_drrs_flush(i915, frontbuffer_bits);
>>   	intel_psr_flush(i915, frontbuffer_bits, origin);
>>   	intel_fbc_flush(i915, frontbuffer_bits, origin);
>> diff --git a/drivers/gpu/drm/i915/display/intel_tdf.h b/drivers/gpu/drm/i915/display/intel_tdf.h
>> new file mode 100644
>> index 000000000000..353cde21f6c2
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/display/intel_tdf.h
>> @@ -0,0 +1,25 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2024 Intel Corporation
>> + */
>> +
>> +#ifndef __INTEL_TDF_H__
>> +#define __INTEL_TDF_H__
>> +
>> +/*
>> + * TDF (Transient-Data-Flush) is needed for Xe2+ where special L3:XD caching can
>> + * be enabled through various PAT index modes. Idea is to use this caching mode
>> + * when for example rendering onto the display surface, with the promise that
>> + * KMD will ensure transient cache entries are always flushed by the time we do
>> + * the display flip, since display engine is never coherent with CPU/GPU caches.
>> + */
>> +
>> +struct drm_i915_private;
>> +
>> +#ifdef I915
>> +static inline void intel_td_flush(struct drm_i915_private *i915) {}
>> +#else
>> +void intel_td_flush(struct drm_i915_private *i915);
>> +#endif
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index 6015c9e41f24..97a8674cdd76 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -198,7 +198,8 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
>>   	display/xe_dsb_buffer.o \
>>   	display/xe_fb_pin.o \
>>   	display/xe_hdcp_gsc.o \
>> -	display/xe_plane_initial.o
>> +	display/xe_plane_initial.o \
>> +	display/xe_tdf.o
>>   
>>   # SOC code shared with i915
>>   xe-$(CONFIG_DRM_XE_DISPLAY) += \
>> diff --git a/drivers/gpu/drm/xe/display/xe_tdf.c b/drivers/gpu/drm/xe/display/xe_tdf.c
>> new file mode 100644
>> index 000000000000..2c0d4e144e09
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/display/xe_tdf.c
>> @@ -0,0 +1,13 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2024 Intel Corporation
>> + */
>> +
>> +#include "xe_device.h"
>> +#include "intel_display_types.h"
>> +#include "intel_tdf.h"
>> +
>> +void intel_td_flush(struct drm_i915_private *i915)
>> +{
>> +	xe_device_td_flush(i915);
>> +}
>> -- 
>> 2.25.1
>>

Matthew Auld April 16, 2024, 7:40 a.m. UTC | #4

On 15/04/2024 19:14, Matt Roper wrote:
> On Mon, Apr 15, 2024 at 10:07:32AM -0700, Matt Roper wrote:
>> On Mon, Apr 15, 2024 at 01:44:22PM +0530, Balasubramani Vivekanandan wrote:
>>> From: Matthew Auld <matthew.auld@intel.com>
>>>
>>> Perform manual transient cache flush prior to flip and at the end of
>>> frontbuffer_flush. This is needed to ensure display engine doesn't see
>>> garbage if the surface is L3:XD dirty.
>>>
>>> Testcase: igt@xe-pat@display-vs-wb-transient
>>
>> Has the IGT patch for this been sent yet?  If not, we should probably
>> make sure that happens soon, and then use the CI Test-with: thing if
>> there winds up being another revision of this series so that this will
>> be included in the CI results.
> 
> Oh, it looks like this test already landed back in early March; I just
> didn't look back far enough in the git history originally.  You can
> ignore this comment.

Yeah, my thinking was to upstream the IGT bits early for LNL, since it 
is still applicable there. Only difference is we are verifying we don't 
need the TDF on that platform, whereas on BMG the test will only pass 
with this patch.

> 
> 
> Matt
> 
>>
>> Anyway, the changes here look good to me,
>>
>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>>
>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>>> Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>>> Acked-by: Nirmoy Das <nirmoy.das@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_display.c  |  3 +++
>>>   .../gpu/drm/i915/display/intel_frontbuffer.c  |  2 ++
>>>   drivers/gpu/drm/i915/display/intel_tdf.h      | 25 +++++++++++++++++++
>>>   drivers/gpu/drm/xe/Makefile                   |  3 ++-
>>>   drivers/gpu/drm/xe/display/xe_tdf.c           | 13 ++++++++++
>>>   5 files changed, 45 insertions(+), 1 deletion(-)
>>>   create mode 100644 drivers/gpu/drm/i915/display/intel_tdf.h
>>>   create mode 100644 drivers/gpu/drm/xe/display/xe_tdf.c
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>> index 67697d9a559c..4fc46edcb4ad 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -110,6 +110,7 @@
>>>   #include "intel_sdvo.h"
>>>   #include "intel_snps_phy.h"
>>>   #include "intel_tc.h"
>>> +#include "intel_tdf.h"
>>>   #include "intel_tv.h"
>>>   #include "intel_vblank.h"
>>>   #include "intel_vdsc.h"
>>> @@ -7242,6 +7243,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>>>   
>>>   	intel_atomic_commit_fence_wait(state);
>>>   
>>> +	intel_td_flush(dev_priv);
>>> +
>>>   	drm_atomic_helper_wait_for_dependencies(&state->base);
>>>   	drm_dp_mst_atomic_wait_for_dependencies(&state->base);
>>>   	intel_atomic_global_state_wait_for_dependencies(state);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
>>> index 2ea37c0414a9..4923c340a0b6 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
>>> @@ -65,6 +65,7 @@
>>>   #include "intel_fbc.h"
>>>   #include "intel_frontbuffer.h"
>>>   #include "intel_psr.h"
>>> +#include "intel_tdf.h"
>>>   
>>>   /**
>>>    * frontbuffer_flush - flush frontbuffer
>>> @@ -93,6 +94,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
>>>   	trace_intel_frontbuffer_flush(i915, frontbuffer_bits, origin);
>>>   
>>>   	might_sleep();
>>> +	intel_td_flush(i915);
>>>   	intel_drrs_flush(i915, frontbuffer_bits);
>>>   	intel_psr_flush(i915, frontbuffer_bits, origin);
>>>   	intel_fbc_flush(i915, frontbuffer_bits, origin);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_tdf.h b/drivers/gpu/drm/i915/display/intel_tdf.h
>>> new file mode 100644
>>> index 000000000000..353cde21f6c2
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/display/intel_tdf.h
>>> @@ -0,0 +1,25 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2024 Intel Corporation
>>> + */
>>> +
>>> +#ifndef __INTEL_TDF_H__
>>> +#define __INTEL_TDF_H__
>>> +
>>> +/*
>>> + * TDF (Transient-Data-Flush) is needed for Xe2+ where special L3:XD caching can
>>> + * be enabled through various PAT index modes. Idea is to use this caching mode
>>> + * when for example rendering onto the display surface, with the promise that
>>> + * KMD will ensure transient cache entries are always flushed by the time we do
>>> + * the display flip, since display engine is never coherent with CPU/GPU caches.
>>> + */
>>> +
>>> +struct drm_i915_private;
>>> +
>>> +#ifdef I915
>>> +static inline void intel_td_flush(struct drm_i915_private *i915) {}
>>> +#else
>>> +void intel_td_flush(struct drm_i915_private *i915);
>>> +#endif
>>> +
>>> +#endif
>>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>>> index 6015c9e41f24..97a8674cdd76 100644
>>> --- a/drivers/gpu/drm/xe/Makefile
>>> +++ b/drivers/gpu/drm/xe/Makefile
>>> @@ -198,7 +198,8 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
>>>   	display/xe_dsb_buffer.o \
>>>   	display/xe_fb_pin.o \
>>>   	display/xe_hdcp_gsc.o \
>>> -	display/xe_plane_initial.o
>>> +	display/xe_plane_initial.o \
>>> +	display/xe_tdf.o
>>>   
>>>   # SOC code shared with i915
>>>   xe-$(CONFIG_DRM_XE_DISPLAY) += \
>>> diff --git a/drivers/gpu/drm/xe/display/xe_tdf.c b/drivers/gpu/drm/xe/display/xe_tdf.c
>>> new file mode 100644
>>> index 000000000000..2c0d4e144e09
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/xe/display/xe_tdf.c
>>> @@ -0,0 +1,13 @@
>>> +// SPDX-License-Identifier: MIT
>>> +/*
>>> + * Copyright © 2024 Intel Corporation
>>> + */
>>> +
>>> +#include "xe_device.h"
>>> +#include "intel_display_types.h"
>>> +#include "intel_tdf.h"
>>> +
>>> +void intel_td_flush(struct drm_i915_private *i915)
>>> +{
>>> +	xe_device_td_flush(i915);
>>> +}
>>> -- 
>>> 2.25.1
>>>
>>
>> -- 
>> Matt Roper
>> Graphics Software Engineer
>> Linux GPU Platform Enablement
>> Intel Corporation
>

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 67697d9a559c..4fc46edcb4ad 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -110,6 +110,7 @@ 
 #include "intel_sdvo.h"
 #include "intel_snps_phy.h"
 #include "intel_tc.h"
+#include "intel_tdf.h"
 #include "intel_tv.h"
 #include "intel_vblank.h"
 #include "intel_vdsc.h"
@@ -7242,6 +7243,8 @@  static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 
 	intel_atomic_commit_fence_wait(state);
 
+	intel_td_flush(dev_priv);
+
 	drm_atomic_helper_wait_for_dependencies(&state->base);
 	drm_dp_mst_atomic_wait_for_dependencies(&state->base);
 	intel_atomic_global_state_wait_for_dependencies(state);
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index 2ea37c0414a9..4923c340a0b6 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -65,6 +65,7 @@ 
 #include "intel_fbc.h"
 #include "intel_frontbuffer.h"
 #include "intel_psr.h"
+#include "intel_tdf.h"
 
 /**
  * frontbuffer_flush - flush frontbuffer
@@ -93,6 +94,7 @@  static void frontbuffer_flush(struct drm_i915_private *i915,
 	trace_intel_frontbuffer_flush(i915, frontbuffer_bits, origin);
 
 	might_sleep();
+	intel_td_flush(i915);
 	intel_drrs_flush(i915, frontbuffer_bits);
 	intel_psr_flush(i915, frontbuffer_bits, origin);
 	intel_fbc_flush(i915, frontbuffer_bits, origin);
diff --git a/drivers/gpu/drm/i915/display/intel_tdf.h b/drivers/gpu/drm/i915/display/intel_tdf.h
new file mode 100644
index 000000000000..353cde21f6c2
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_tdf.h
@@ -0,0 +1,25 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 Intel Corporation
+ */
+
+#ifndef __INTEL_TDF_H__
+#define __INTEL_TDF_H__
+
+/*
+ * TDF (Transient-Data-Flush) is needed for Xe2+ where special L3:XD caching can
+ * be enabled through various PAT index modes. Idea is to use this caching mode
+ * when for example rendering onto the display surface, with the promise that
+ * KMD will ensure transient cache entries are always flushed by the time we do
+ * the display flip, since display engine is never coherent with CPU/GPU caches.
+ */
+
+struct drm_i915_private;
+
+#ifdef I915
+static inline void intel_td_flush(struct drm_i915_private *i915) {}
+#else
+void intel_td_flush(struct drm_i915_private *i915);
+#endif
+
+#endif
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 6015c9e41f24..97a8674cdd76 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -198,7 +198,8 @@  xe-$(CONFIG_DRM_XE_DISPLAY) += \
 	display/xe_dsb_buffer.o \
 	display/xe_fb_pin.o \
 	display/xe_hdcp_gsc.o \
-	display/xe_plane_initial.o
+	display/xe_plane_initial.o \
+	display/xe_tdf.o
 
 # SOC code shared with i915
 xe-$(CONFIG_DRM_XE_DISPLAY) += \
diff --git a/drivers/gpu/drm/xe/display/xe_tdf.c b/drivers/gpu/drm/xe/display/xe_tdf.c
new file mode 100644
index 000000000000..2c0d4e144e09
--- /dev/null
+++ b/drivers/gpu/drm/xe/display/xe_tdf.c
@@ -0,0 +1,13 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 Intel Corporation
+ */
+
+#include "xe_device.h"
+#include "intel_display_types.h"
+#include "intel_tdf.h"
+
+void intel_td_flush(struct drm_i915_private *i915)
+{
+	xe_device_td_flush(i915);
+}

[v3,20/21] drm/i915/display: perform transient flush

Commit Message

Comments

Patch