[Intel-gfx] [PATCH v2 2/2] drm/i915/icl: Add IOCTL for getting MOCS table version

[Daniele Ceraolo Spurio]

On 22/10/18 10:13, Tomasz Lis wrote:
> For Icelake and above, MOCS table for each platform is published
> within bspec. The table is versioned, and new entries are assigned
> a version number. Existing entries do not change and their version
> is constant.
> 
> This introduces a parameter which allows getting max version number
> of the MOCS entries currently supported, ie. value of 2 would mean
> only version 1 and version 2 entries are initialized and can be used
> by the user mode clients.
> 
> BSpec: 34007
> Signed-off-by: Tomasz Lis <tomasz.lis at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> Cc: Zhi A Wang <zhi.a.wang at intel.com>
> Cc: Anuj Phogat <anuj.phogat at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.c   |  6 ++++++
>   drivers/gpu/drm/i915/intel_mocs.c | 12 ++++++++++++
>   drivers/gpu/drm/i915/intel_mocs.h |  1 +
>   include/uapi/drm/i915_drm.h       | 11 +++++++++++
>   4 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index baac35f..92fa8fd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -53,6 +53,7 @@
>   #include "i915_vgpu.h"
>   #include "intel_drv.h"
>   #include "intel_uc.h"
> +#include "intel_mocs.h"
>   
>   static struct drm_driver driver;
>   
> @@ -444,6 +445,11 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data,
>   	case I915_PARAM_MMAP_GTT_COHERENT:
>   		value = INTEL_INFO(dev_priv)->has_coherent_ggtt;
>   		break;
> +	case I915_PARAM_MOCS_TABLE_VERSION:
> +		value = intel_mocs_table_version(dev_priv);
> +		if (!value)
> +			return -ENODEV;

Do we really want to return -ENODEV for platforms that do have a MOCS 
table programmed, but the table is not one versioned in the specs (i.e. 
Gen9-10)? I think returning "0" for those to indicate "kernel-defined 
table" would be ok and we could limit -ENODEV for platforms that don't 
have a table at all. But what wins is what the callers of the ioctl 
would like to get from the kernel ;)

> +		break;
>   	default:
>   		DRM_DEBUG("Unknown parameter %d\n", param->param);
>   		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index dc34e83..fc1e98b 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -469,6 +469,18 @@ static i915_reg_t mocs_register(enum intel_engine_id engine_id, int index)
>   }
>   
>   /**
> + * intel_mocs_table_version() - get version of mocs table implementation
> + * @i915: i915 device struct.
> + */
> +int intel_mocs_table_version(struct drm_i915_private *i915)
> +{
> +	if (IS_ICELAKE(i915))
> +		return 1;

Can we add this version value as a define above the table, to keep them 
close to each other?

If we agree on my suggestion above to differentiate between no table at 
all (< 0), kernel-defined table (= 0) and spec-defined versioned table 
(> 0), it might also be useful to store the version with the table info 
in drm_i915_mocs_table and then have intel_mocs_table_version call 
get_mocs_settings(), e.g:

int intel_mocs_table_version(struct drm_i915_private *i915)
{
	struct drm_i915_mocs_table table;

	if (!get_mocs_settings(i915, &table))
		return -ENODEV;

	return table->version;
}

Thanks,
Daniele

> +	else
> +		return 0;
> +}
> +
> +/**
>    * intel_mocs_init_engine() - emit the mocs control table
>    * @engine:	The engine for whom to emit the registers.
>    *
> diff --git a/drivers/gpu/drm/i915/intel_mocs.h b/drivers/gpu/drm/i915/intel_mocs.h
> index d89080d..dc1d64a 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.h
> +++ b/drivers/gpu/drm/i915/intel_mocs.h
> @@ -55,5 +55,6 @@
>   int intel_rcs_context_init_mocs(struct i915_request *rq);
>   void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv);
>   void intel_mocs_init_engine(struct intel_engine_cs *engine);
> +int intel_mocs_table_version(struct drm_i915_private *i915);
>   
>   #endif
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 298b2e1..16aafc4 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -559,6 +559,17 @@ typedef struct drm_i915_irq_wait {
>    */
>   #define I915_PARAM_MMAP_GTT_COHERENT	52
>   
> +/*
> + * Query MOCS table version used during hardware initialization.
> + *
> + * The MOCS table for each platform is published as part of bspec. Entries in
> + * the table are supposed to never be modified, but new enties are added, making
> + * more indexes in the table valid. This parameter informs which version
> + * of the table was used to initialize the currently used graphics hardware,
> + * and therefore which MOCS indexes are useable.
> + */
> +#define I915_PARAM_MOCS_TABLE_VERSION	53
> +
>   typedef struct drm_i915_getparam {
>   	__s32 param;
>   	/*
>