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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2018-10-22 20:18, Daniele Ceraolo Spurio wrote:


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@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
Cc: Zhi A Wang <zhi.a.wang@xxxxxxxxx>
Cc: Anuj Phogat <anuj.phogat@xxxxxxxxx>
---
  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
That does sound reasonable. And I agree we should really ask userland what exactly they want. Right now there is no request from any UMD for such interface; we will get back to this patch when it is requested. Joonas also mentioned there is no need for an interface which always returns "1", and is expected to return only that value for future platforms as well. That's a good argument.

I will remove this patch from the current series.

-Tomasz

+    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;
      /*


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux