[Intel-xe] [PATCH v4 1/9] drm/xe/xe2: Determine bios enablement for flat ccs on igfx

[Matt Roper]

On Wed, Dec 06, 2023 at 10:01:18AM +0530, Himal Prasad Ghimiray wrote:
> If bios disables flat ccs on igfx make has_flat_ccs as 0 and notify
> via drm_info.
> 
> Bspec:59255

This bspec reference is just the register detail page for
FLAT_CCS_BASE_ADDR.  Is there a better overview page you can provide as
reference instead or as well?  I can't find anything that indicates the
'enable' bit is only relevant to integrated graphics platforms.  In fact
other overview pages like 58797 or 67090 seem to imply that it would
apply to all Xe2 platforms.

Also, how does this enable bit relate to the one in register 0x4910?
Are they always set the same way (enabled or disabled) by the BIOS?


> 
> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_gt_regs.h |  3 +++
>  drivers/gpu/drm/xe/xe_device.c       | 29 ++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> index d318ec0efd7d..5979ec82ea31 100644
> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> @@ -145,6 +145,9 @@
>  #define XEHP_SLICE_COMMON_ECO_CHICKEN1		XE_REG_MCR(0x731c, XE_REG_OPTION_MASKED)
>  #define   MSC_MSAA_REODER_BUF_BYPASS_DISABLE	REG_BIT(14)
>  
> +#define XE2_FLAT_CCS_BASE_RANGE_LOWER		XE_REG_MCR(0x8800)

This register definition seems to be mis-sorted; it shouldn't be before
0x83a4.

> +#define   XE2_FLAT_CCS_ENABLE			REG_BIT(0)
> +
>  #define VF_PREEMPTION				XE_REG(0x83a4, XE_REG_OPTION_MASKED)
>  #define   PREEMPTION_VERTEX_COUNT		REG_GENMASK(15, 0)
>  
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 8423c817111b..400fa1ac6168 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -16,6 +16,7 @@
>  #include <drm/xe_drm.h>
>  
>  #include "regs/xe_regs.h"
> +#include "regs/xe_gt_regs.h"
>  #include "xe_bo.h"
>  #include "xe_debugfs.h"
>  #include "xe_display.h"
> @@ -26,6 +27,7 @@
>  #include "xe_exec.h"
>  #include "xe_ggtt.h"
>  #include "xe_gt.h"
> +#include "xe_gt_mcr.h"
>  #include "xe_irq.h"
>  #include "xe_mmio.h"
>  #include "xe_module.h"
> @@ -392,6 +394,29 @@ int xe_device_probe_early(struct xe_device *xe)
>  	return 0;
>  }
>  
> +static int xe_device_set_has_flat_ccs(struct  xe_device *xe)
> +{
> +	u32 reg;
> +	int err;
> +
> +	if (IS_DGFX(xe) || GRAPHICS_VER(xe) < 20 || !xe->info.has_flat_ccs)
> +		return 0;

As noted above, I don't see anything that says this needs to be igpu
specific; it seems this should be the general programming for all future
platforms, igpu or dgpu.  If nothing else, we should probably drop this
condition now and only add it back in in the future when we actually add
support for an Xe2 dgpu and know it's the right thing to do.

> +
> +	struct xe_gt *gt = xe_root_mmio_gt(xe);
> +
> +	err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);

This forcewake never gets released.

> +	if (err)
> +		return err;
> +
> +	reg = xe_gt_mcr_unicast_read_any(gt, XE2_FLAT_CCS_BASE_RANGE_LOWER);
> +	xe->info.has_flat_ccs = (reg & XE2_FLAT_CCS_ENABLE);
> +
> +	if (!xe->info.has_flat_ccs)
> +		drm_info(&xe->drm,
> +			 "Flat CCS has been disabled in bios, May lead to performance impact");

I don't think this needs to be a drm_info; a drm_dbg would probably be
fine.  Otherwise end users may be alarmed when they see this showing up
in the dmesg and don't know what it means or even what kind of
"performance" we're talking about.


Matt

> +	return 0;
> +}
> +
>  int xe_device_probe(struct xe_device *xe)
>  {
>  	struct xe_tile *tile;
> @@ -447,6 +472,10 @@ int xe_device_probe(struct xe_device *xe)
>  			goto err_irq_shutdown;
>  	}
>  
> +	err = xe_device_set_has_flat_ccs(xe);
> +	if (err)
> +		return err;
> +
>  	err = xe_mmio_probe_vram(xe);
>  	if (err)
>  		goto err_irq_shutdown;
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation