From: "Souza, Jose" <jose.souza@intel.com>
To: "intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"Yokoyama, Caz" <caz.yokoyama@intel.com>
Cc: "Sripada, Radhakrishna" <radhakrishna.sripada@intel.com>,
"Atwood, Matthew S" <matthew.s.atwood@intel.com>
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Add struct to hold IP version
Date: Thu, 28 Oct 2021 21:08:08 +0000 [thread overview]
Message-ID: <e4a994711cf0bb0d580269eda56ab0bcdad98f87.camel@intel.com> (raw)
In-Reply-To: <6349cf75e1a674288250a137b09911ea2349fc41.camel@intel.com>
On Fri, 2021-10-22 at 21:26 +0000, Yokoyama, Caz wrote:
> On Wed, 2021-10-20 at 19:19 +0000, Souza, Jose wrote:
> > On Wed, 2021-10-20 at 15:00 +0000, Yokoyama, Caz wrote:
> > > On Tue, 2021-10-19 at 17:23 -0700, José Roberto de Souza wrote:
> > > > Adding a structure to standardize access to IP versioning as
> > > > future
> > > > platforms will have this information populated at runtime.
> > > >
> > > > The constant platform display version is not using this new
> > > > struct
> > > > but
> > > > the runtime variant will definitely use it.
> > > >
> > > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > Cc: Matt Atwood <matthew.s.atwood@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.c | 2 +-
> > > > drivers/gpu/drm/i915/i915_drv.h | 12 ++++++------
> > > > drivers/gpu/drm/i915/i915_pci.c | 18 +++++++++--
> > > > ----
> > > > ---
> > > > drivers/gpu/drm/i915/intel_device_info.c | 19 ++++++++++++-
> > > > ----
> > > > --
> > > > drivers/gpu/drm/i915/intel_device_info.h | 12 ++++++++----
> > > > .../gpu/drm/i915/selftests/mock_gem_device.c | 2 +-
> > > > 6 files changed, 37 insertions(+), 28 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > > b/drivers/gpu/drm/i915/i915_drv.c
> > > > index 1e5b75ae99329..bdf85d202c55c 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -808,7 +808,7 @@ int i915_driver_probe(struct pci_dev *pdev,
> > > > const
> > > > struct pci_device_id *ent)
> > > > return PTR_ERR(i915);
> > > >
> > > > /* Disable nuclear pageflip by default on pre-ILK */
> > > > - if (!i915->params.nuclear_pageflip && match_info-
> > > > > graphics_ver
> > > > < 5)
> > > > + if (!i915->params.nuclear_pageflip && match_info-
> > > > > graphics.ver
> > > > < 5)
> > > I don't find any difference on this and the similar modifications
> > > below. Am I missing something?
> >
> > Changing u8 graphics_ver to struct ip_version that contains a member
> > called ver.
> > So only changing '_' to '.' in most places.
> >
> > > -caz
> > >
> > > > i915->drm.driver_features &= ~DRIVER_ATOMIC;
> > > >
> > > > /*
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 12256218634f4..26b6e2b8bb5e8 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -1327,15 +1327,15 @@ static inline struct drm_i915_private
> > > > *pdev_to_i915(struct pci_dev *pdev)
> > > >
> > > > #define IP_VER(ver, rel) ((ver) << 8 | (rel))
> > > >
> > > > -#define GRAPHICS_VER(i915) (INTEL_INFO(i915)-
> > > > > graphics_ver)
> > > > -#define
> > > > GRAPHICS_VER_FULL(i915) IP_VER(INTEL_INFO(i915)
> > > > ->graphics_ver, \
> > > > - INTEL_INFO(i915)-
> > > > > graphics_rel)
> > > > +#define GRAPHICS_VER(i915) (INTEL_INFO(i915)-
> > > > > graphics.ver)
> > > > +#define
> > > > GRAPHICS_VER_FULL(i915) IP_VER(INTEL_INFO(i915)
> > > > ->graphics.ver, \
> > > > + INTEL_INFO(i915)-
> > > > > graphics.rel)
> > > > #define IS_GRAPHICS_VER(i915, from, until) \
> > > > (GRAPHICS_VER(i915) >= (from) && GRAPHICS_VER(i915) <=
> > > > (until))
> > > >
> > > > -#define MEDIA_VER(i915) (INTEL_INFO(i915)-
> > > > > media_ver)
> > > > -#define MEDIA_VER_FULL(i915) IP_VER(INTEL_INFO(i915)-
> > > > > media_ver, \
> > > > - INTEL_INFO(i915)-
> > > > > media_rel)
> > > > +#define MEDIA_VER(i915) (INTEL_INFO(i915)-
> > > > > media.ver)
> > > > +#define MEDIA_VER_FULL(i915) IP_VER(INTEL_INFO(i915)-
> > > > > media.arch, \
> > > > + INTEL_INFO(i915)-
> > > > > media.rel)
> > > > #define IS_MEDIA_VER(i915, from, until) \
> > > > (MEDIA_VER(i915) >= (from) && MEDIA_VER(i915) <= (until))
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > > > b/drivers/gpu/drm/i915/i915_pci.c
> > > > index 169837de395d3..5e6795853dc31 100644
> > > > --- a/drivers/gpu/drm/i915/i915_pci.c
> > > > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > > > @@ -32,8 +32,8 @@
> > > >
> > > > #define PLATFORM(x) .platform = (x)
> > > > #define GEN(x) \
> > > > - .graphics_ver = (x), \
> > > > - .media_ver = (x), \
> > > > + .graphics.ver = (x), \
> > > > + .media.ver = (x), \
> > > > .display.ver = (x)
> > > >
> > > > #define I845_PIPE_OFFSETS \
> > > > @@ -899,7 +899,7 @@ static const struct intel_device_info
> > > > rkl_info =
> > > > {
> > > > static const struct intel_device_info dg1_info = {
> > > > GEN12_FEATURES,
> > > > DGFX_FEATURES,
> > > > - .graphics_rel = 10,
> > > > + .graphics.rel = 10,
> > > > PLATFORM(INTEL_DG1),
> > > > .pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C) |
> > > > BIT(PIPE_D),
> > > > .require_force_probe = 1,
> > > > @@ -986,8 +986,8 @@ static const struct intel_device_info
> > > > adl_p_info
> > > > = {
> > > > I915_GTT_PAGE_SIZE_2M
> > > >
> > > > #define XE_HP_FEATURES \
> > > > - .graphics_ver = 12, \
> > > > - .graphics_rel = 50, \
> > > > + .graphics.ver = 12, \
> > > > + .graphics.rel = 50, \
> > > > XE_HP_PAGE_SIZES, \
> > > > .dma_mask_size = 46, \
> > > > .has_64bit_reloc = 1, \
> > > > @@ -1005,8 +1005,8 @@ static const struct intel_device_info
> > > > adl_p_info = {
> > > > .ppgtt_type = INTEL_PPGTT_FULL
> > > >
> > > > #define XE_HPM_FEATURES \
> > > > - .media_ver = 12, \
> > > > - .media_rel = 50
> > > > + .media.ver = 12, \
> > > > + .media.rel = 50
> > > >
> > > > __maybe_unused
> > > > static const struct intel_device_info xehpsdv_info = {
> > > > @@ -1030,8 +1030,8 @@ static const struct intel_device_info
> > > > dg2_info
> > > > = {
> > > > XE_HPM_FEATURES,
> > > > XE_LPD_FEATURES,
> > > > DGFX_FEATURES,
> > > > - .graphics_rel = 55,
> > > > - .media_rel = 55,
> > > > + .graphics.rel = 55,
> > > > + .media.rel = 55,
> > > > PLATFORM(INTEL_DG2),
> > > > .platform_engine_mask =
> > > > BIT(RCS0) | BIT(BCS0) |
> > > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > > > b/drivers/gpu/drm/i915/intel_device_info.c
> > > > index 305facedd2841..6e6b317bc33ce 100644
> > > > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > > > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > > > @@ -97,17 +97,22 @@ static const char *iommu_name(void)
> > > > void intel_device_info_print_static(const struct
> > > > intel_device_info
> > > > *info,
> > > > struct drm_printer *p)
> > > > {
> > > > - if (info->graphics_rel)
> > > > - drm_printf(p, "graphics version: %u.%02u\n", info-
> > > > > graphics_ver, info->graphics_rel);
> > > > + if (info->graphics.rel)
> > > > + drm_printf(p, "graphics version: %u.%02u\n", info-
> > > > > graphics.ver,
> > > > + info->graphics.rel);
> > > > else
> > > > - drm_printf(p, "graphics version: %u\n", info-
> > > > > graphics_ver);
> > > > + drm_printf(p, "graphics version: %u\n", info-
> > > > > graphics.ver);
> > > >
> > > > - if (info->media_rel)
> > > > - drm_printf(p, "media version: %u.%02u\n", info-
> > > > > media_ver, info->media_rel);
> > > > + if (info->media.rel)
> > > > + drm_printf(p, "media version: %u.%02u\n", info-
> > > > > media.ver, info->media.rel);
> > > > else
> > > > - drm_printf(p, "media version: %u\n", info-
> > > > > media_ver);
> > > > + drm_printf(p, "media version: %u\n", info-
> > > > > media.ver);
> > > > +
> > > > + if (info->display.rel)
> > > > + drm_printf(p, "display version: %u.%02u\n", info-
> > > > > display.ver, info->display.rel);
> > > > + else
> > > > + drm_printf(p, "display version: %u\n", info-
> > > > > display.ver);
> > > >
> > > > - drm_printf(p, "display version: %u\n", info->display.ver);
> > > > drm_printf(p, "gt: %d\n", info->gt);
> > > > drm_printf(p, "iommu: %s\n", iommu_name());
> > > > drm_printf(p, "memory-regions: %x\n", info-
> > > > > memory_regions);
> > > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> > > > b/drivers/gpu/drm/i915/intel_device_info.h
> > > > index 8e6f48d1eb7bc..669f0d26c3c38 100644
> > > > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > > > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > > > @@ -166,11 +166,14 @@ enum intel_ppgtt_type {
> > > > func(overlay_needs_physical); \
> > > > func(supports_tv);
> > > >
> > > > +struct ip_version {
> > > > + u8 ver;
> > > > + u8 rel;
> > > > +};
> > > > +
> > > > struct intel_device_info {
> > > > - u8 graphics_ver;
> > > > - u8 graphics_rel;
> > > > - u8 media_ver;
> > > > - u8 media_rel;
> > > > + struct ip_version graphics;
> > > > + struct ip_version media;
> > > >
> > > > intel_engine_mask_t platform_engine_mask; /* Engines
> > > > supported
> > > > by the HW */
> > > >
> > > > @@ -200,6 +203,7 @@ struct intel_device_info {
> > > >
> > > > struct {
> > > > u8 ver;
> > > > + u8 rel;
> Even this is mentioned in the comment as "need for runtime variant", I
> have no idea why it is needed because it is copied on i915_drv.c and
> shown by drm_printf() in intel_device_info.c.
It is to make standard access between every IP, right now we already graphics IP with ver and rel(DG2 graphics_rel = 55 while xehpsdv is
graphics_rel=50) but not a display one.
But that will change in future platforms, so making the access the same for every IP.
> If it is really needed,
> Reviewed-by: Caz Yokoyama <caz.yokoyama@intel.com>
> -caz
>
> > > >
> > > > #define DEFINE_FLAG(name) u8 name:1
> > > > DEV_INFO_DISPLAY_FOR_EACH_FLAG(DEFINE_FLAG);
> > > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > index 4f81801468881..9ab3f284d1dd9 100644
> > > > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > @@ -165,7 +165,7 @@ struct drm_i915_private
> > > > *mock_gem_device(void)
> > > > /* Using the global GTT may ask questions about KMS users,
> > > > so
> > > > prepare */
> > > > drm_mode_config_init(&i915->drm);
> > > >
> > > > - mkwrite_device_info(i915)->graphics_ver = -1;
> > > > + mkwrite_device_info(i915)->graphics.ver = -1;
> > > >
> > > > mkwrite_device_info(i915)->page_sizes =
> > > > I915_GTT_PAGE_SIZE_4K |
next prev parent reply other threads:[~2021-10-28 21:08 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-20 0:23 [Intel-gfx] [PATCH 1/3] drm/i915: Add struct to hold IP version José Roberto de Souza
2021-10-20 0:23 ` [Intel-gfx] [PATCH 2/3] drm/i915: Track media IP stepping separated from GT José Roberto de Souza
2021-11-02 7:30 ` Lucas De Marchi
2021-10-20 0:23 ` [Intel-gfx] [PATCH 3/3] drm/i915: Rename GT_STEP to GRAPHICS_STEP José Roberto de Souza
2021-10-20 15:06 ` Yokoyama, Caz
2021-11-02 7:27 ` Lucas De Marchi
2021-10-20 0:37 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915: Add struct to hold IP version Patchwork
2021-10-20 1:08 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-10-20 5:10 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-11-02 20:04 ` Souza, Jose
2021-10-20 9:47 ` [Intel-gfx] [PATCH 1/3] " Jani Nikula
2021-10-20 19:29 ` Souza, Jose
2021-10-21 13:11 ` Jani Nikula
2021-10-22 20:15 ` Lucas De Marchi
2021-10-25 9:04 ` Jani Nikula
2021-11-02 5:33 ` Souza, Jose
2021-10-20 15:00 ` Yokoyama, Caz
2021-10-20 19:19 ` Souza, Jose
2021-10-22 21:26 ` Yokoyama, Caz
2021-10-28 21:08 ` Souza, Jose [this message]
2021-11-01 14:29 ` Yokoyama, Caz
2021-11-02 7:32 ` Lucas De Marchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e4a994711cf0bb0d580269eda56ab0bcdad98f87.camel@intel.com \
--to=jose.souza@intel.com \
--cc=caz.yokoyama@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=matthew.s.atwood@intel.com \
--cc=radhakrishna.sripada@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox