Re: [Intel-xe] [PATCH 3/6] drm/xe/mocs: Drop xe_mocs_info_index

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Thu, Feb 16, 2023 at 03:17:21PM -0800, Matt Roper wrote:
>The values in the xe_mocs_info_index enum only match old pre-gen12
>hardware not supported by the Xe driver.
>
>The only usage of this enum was to set a default value for
>info->unused_entries_index, but this is unnecessary since every platform
>in the subsequent switch statement sets a proper platform-specific value
>(and the XE_MOCS_PTE default doesn't even make sense since the hardware
>dropped the "use PAT settings" capability in gen12).
>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>---
> drivers/gpu/drm/xe/xe_mocs.c | 30 ++----------------------------
> 1 file changed, 2 insertions(+), 28 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_mocs.c b/drivers/gpu/drm/xe/xe_mocs.c
>index ec89ff3ac29b..583e198af88d 100644
>--- a/drivers/gpu/drm/xe/xe_mocs.c
>+++ b/drivers/gpu/drm/xe/xe_mocs.c
>@@ -23,30 +23,6 @@ static inline void mocs_dbg(const struct drm_device *dev,
> { /* noop */ }
> #endif
>
>-/*
>- * MOCS indexes used for GPU surfaces, defining the cacheability of the
>- * surface data and the coherency for this data wrt. CPU vs. GPU accesses.
>- */
>-enum xe_mocs_info_index {
>-	/*
>-	 * Not cached anywhere, coherency between CPU and GPU accesses is
>-	 * guaranteed.
>-	 */
>-	XE_MOCS_UNCACHED,
>-	/*
>-	 * Cacheability and coherency controlled by the kernel automatically
>-	 * based on the xxxx  IOCTL setting and the current
>-	 * usage of the surface (used for display scanout or not).
>-	 */
>-	XE_MOCS_PTE,
>-	/*
>-	 * Cached in all GPU caches available on the platform.
>-	 * Coherency between CPU and GPU accesses to the surface is not
>-	 * guaranteed without extra synchronization.
>-	 */
>-	XE_MOCS_CACHED,
>-};
>-
> enum {
> 	HAS_GLOBAL_MOCS = BIT(0),
> 	HAS_RENDER_L3CC = BIT(1),
>@@ -341,7 +317,6 @@ static unsigned int get_mocs_settings(struct xe_device *xe,
>
> 	memset(info, 0, sizeof(struct xe_mocs_info));
>
>-	info->unused_entries_index = XE_MOCS_PTE;

since leaving this as 0 should never be valid, maybe add a warning after
the switch for the cases someoneone forgot to set it appropriately?

with that,

	Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

thanks
Lucas De Marchi


> 	switch (xe->info.platform) {
> 	case XE_PVC:
> 		info->size = ARRAY_SIZE(pvc_mocs_desc);
>@@ -406,9 +381,8 @@ static unsigned int get_mocs_settings(struct xe_device *xe,
> }
>
> /*
>- * Get control_value from MOCS entry taking into account when it's not used
>- * then if unused_entries_index is non-zero then its value will be returned
>- * otherwise XE_MOCS_PTE's value is returned in this case.
>+ * Get control_value from MOCS entry.  If the table entry is not defined, the
>+ * settings from unused_entries_index will be returned.
>  */
> static u32 get_entry_control(const struct xe_mocs_info *info,
> 			     unsigned int index)
>-- 
>2.39.1
>