Re: [PATCH v2 2/2] drm/i915: disable fbc due to Wa_16023588340

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 02 Jul 2024, Matthew Auld <matthew.auld@intel.com> wrote:
> Hi,
>
> On 02/07/2024 09:17, Jani Nikula wrote:
>> On Mon, 01 Jul 2024, Matthew Auld <matthew.auld@intel.com> wrote:
>>> On BMG-G21 we need to disable fbc due to complications around the WA.
>>>
>>> v2:
>>>   - Try to handle with i915_drv.h and compat layer. (Rodrigo)
>>>
>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>>> Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>> Cc: Vinod Govindapillai <vinod.govindapillai@intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Cc: intel-gfx@lists.freedesktop.org
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_fbc.c          | 5 +++++
>>>   drivers/gpu/drm/i915/i915_drv.h                   | 2 ++
>>>   drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h | 5 +++++
>>>   3 files changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
>>> index 67116c9f1464..60131de77b4c 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>>> @@ -1237,6 +1237,11 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state,
>>>   		return 0;
>>>   	}
>>>   
>>> +	if (DISPLAY_NEEDS_WA_16023588340(i915)) {
>>> +		plane_state->no_fbc_reason = "Wa_16023588340";
>>> +		return 0;
>>> +	}
>>> +
>>>   	/* WaFbcTurnOffFbcWhenHyperVisorIsUsed:skl,bxt */
>>>   	if (i915_vtd_active(i915) && (IS_SKYLAKE(i915) || IS_BROXTON(i915))) {
>>>   		plane_state->no_fbc_reason = "VT-d enabled";
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index d7723dd11c80..816a01fda3fe 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -762,4 +762,6 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>>>   #define HAS_LMEMBAR_SMEM_STOLEN(i915) (!HAS_LMEM(i915) && \
>>>   				       GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
>>>   
>>> +#define DISPLAY_NEEDS_WA_16023588340(i915)	false
>>> +
>> 
>> Display feature macros don't belong in i915_drv.h.
>
> There is also the v1 here:
> https://patchwork.freedesktop.org/patch/599900/?series=135061&rev=1
>
> Where feedback was to handle it in i915_drv.h. Is the v1 method 
> acceptable here? Do you have an alternative suggestion?

Based on Lucas' reply, looks like it needs to be a function in a .c file
anyway.

One of my goals lately has been to avoid including i915_drv.h from
display code altogether. We're obviously not even close yet, but
anything you put there is just another problem for me to solve. So I'd
like you to solve it, not me. ;)

BR,
Jani.

>
>> 
>> BR,
>> Jani.
>> 
>>>   #endif
>>> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>>> index 2feedddf1e40..a4256144dff7 100644
>>> --- a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>>> +++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
>>> @@ -15,6 +15,9 @@
>>>   #include "i915_utils.h"
>>>   #include "intel_runtime_pm.h"
>>>   #include "xe_device_types.h"
>>> +#include "xe_wa.h"
>>> +
>>> +#include <generated/xe_wa_oob.h>
>>>   
>>>   static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
>>>   {
>>> @@ -120,6 +123,8 @@ struct i915_sched_attr {
>>>   
>>>   #define FORCEWAKE_ALL XE_FORCEWAKE_ALL
>>>   
>>> +#define DISPLAY_NEEDS_WA_16023588340(xe)	XE_WA(xe_root_mmio_gt(xe), 16023588340)
>>> +
>>>   #ifdef CONFIG_ARM64
>>>   /*
>>>    * arm64 indirectly includes linux/rtc.h,
>> 

-- 
Jani Nikula, Intel