On Fri, Mar 15, 2019 at 02:21:57PM +0000, Tvrtko Ursulin wrote:
>
> On 15/03/2019 14:09, Ville Syrjälä wrote:
> > On Fri, Mar 15, 2019 at 12:26:33PM +0000, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >>
> >> Concept of a sub-platform already exist in our code (like ULX and ULT
> >> platform variants and similar),implemented via the macros which check a
> >> list of device ids to determine a match.
> >>
> >> With this patch we consolidate device ids checking into a single function
> >> called during early driver load.
> >>
> >> A few low bits in the platform mask are reserved for sub-platform
> >> identification and defined as a per-platform namespace.
> >>
> >> At the same time it future proofs the platform_mask handling by preparing
> >> the code for easy extending, and tidies the very verbose WARN strings
> >> generated when IS_PLATFORM macros are embedded into a WARN type
> >> statements.
> >>
> >> The approach is also beneficial to driver size, with an combined shrink of
> >> code and strings of around 1.7 kiB.
> >>
> >> v2: Fixed IS_SUBPLATFORM. Updated commit msg.
> >> v3: Chris was right, there is an ordering problem.
> >>
> >> v4:
> >> * Catch-up with new sub-platforms.
> >> * Rebase for RUNTIME_INFO.
> >> * Drop subplatform mask union tricks and convert platform_mask to an
> >> array for extensibility.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >> Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> >> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> >> Cc: Jose Souza <jose.souza@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.c | 7 +-
> >> drivers/gpu/drm/i915/i915_drv.h | 110 +++++++++++++++--------
> >> drivers/gpu/drm/i915/i915_pci.c | 2 +-
> >> drivers/gpu/drm/i915/intel_device_info.c | 79 ++++++++++++++++
> >> drivers/gpu/drm/i915/intel_device_info.h | 28 +++++-
> >> 5 files changed, 179 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> >> index 0d743907e7bc..3218350cd225 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.c
> >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> >> @@ -863,6 +863,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
> >> if (i915_inject_load_failure())
> >> return -ENODEV;
> >>
> >> + intel_device_info_subplatform_init(dev_priv);
> >> +
> >> spin_lock_init(&dev_priv->irq_lock);
> >> spin_lock_init(&dev_priv->gpu_error.lock);
> >> mutex_init(&dev_priv->backlight_lock);
> >> @@ -1752,10 +1754,11 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv)
> >> if (drm_debug & DRM_UT_DRIVER) {
> >> struct drm_printer p = drm_debug_printer("i915 device info:");
> >>
> >> - drm_printf(&p, "pciid=0x%04x rev=0x%02x platform=%s gen=%i\n",
> >> + drm_printf(&p, "pciid=0x%04x rev=0x%02x platform=%s (%x) gen=%i\n",
> >> INTEL_DEVID(dev_priv),
> >> INTEL_REVID(dev_priv),
> >> intel_platform_name(INTEL_INFO(dev_priv)->platform),
> >> + RUNTIME_INFO(dev_priv)->platform_mask[INTEL_INFO(dev_priv)->platform / (BITS_PER_TYPE(RUNTIME_INFO(dev_priv)->platform_mask[0]) - INTEL_SUBPLATFORM_BITS)],
> >> INTEL_GEN(dev_priv));
> >>
> >> intel_device_info_dump_flags(INTEL_INFO(dev_priv), &p);
> >> @@ -1798,8 +1801,6 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
> >> memcpy(device_info, match_info, sizeof(*device_info));
> >> RUNTIME_INFO(i915)->device_id = pdev->device;
> >>
> >> - BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
> >> - BITS_PER_TYPE(device_info->platform_mask));
> >> BUG_ON(device_info->gen > BITS_PER_TYPE(device_info->gen_mask));
> >>
> >> return i915;
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index dccb6006aabf..34282cf66cb0 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -2281,7 +2281,46 @@ static inline unsigned int i915_sg_segment_size(void)
> >> #define IS_REVID(p, since, until) \
> >> (INTEL_REVID(p) >= (since) && INTEL_REVID(p) <= (until))
> >>
> >> -#define IS_PLATFORM(dev_priv, p) (INTEL_INFO(dev_priv)->platform_mask & BIT(p))
> >> +#define __IS_PLATFORM(dev_priv, p) \
> >> +({ \
> >> + const unsigned int pbits__ = \
> >> + BITS_PER_TYPE(RUNTIME_INFO(dev_priv)->platform_mask[0]) - \
> >> + INTEL_SUBPLATFORM_BITS; \
> >> + const unsigned int pi__ = (p) / pbits__; \
> >> + const unsigned int pb__ = (p) % pbits__ + INTEL_SUBPLATFORM_BITS; \
> >> +\
> >> + BUILD_BUG_ON(!__builtin_constant_p(p)); \
> >> +\
> >> + (RUNTIME_INFO(dev_priv)->platform_mask[pi__] & BIT(pb__)); \
> >> +})
> >> +
> >> +#define __IS_SUBPLATFORM(dev_priv, p, s) \
> >> +({ \
> >> + const unsigned int pbits__ = \
> >> + BITS_PER_TYPE(RUNTIME_INFO(dev_priv)->platform_mask[0]) - \
> >> + INTEL_SUBPLATFORM_BITS; \
> >> + const unsigned int pi__ = (p) / pbits__; \
> >> + const unsigned int pb__ = (p) % pbits__ + INTEL_SUBPLATFORM_BITS; \
> >> +\
> >> + BUILD_BUG_ON(!__builtin_constant_p(p)); \
> >> + BUILD_BUG_ON(!__builtin_constant_p(s)); \
> >> + BUILD_BUG_ON((s) >= INTEL_SUBPLATFORM_BITS); \
> >> +\
> >> + (RUNTIME_INFO(dev_priv)->platform_mask[pi__] & (BIT(pb__) | BIT(s))); \
> >> +})
> >> +
> >> +static inline bool
> >> +IS_PLATFORM(const struct drm_i915_private *i915, enum intel_platform p)
> >> +{
> >> + return __IS_PLATFORM(i915, p);
> >> +}
> >> +
> >> +static inline bool
> >> +IS_SUBPLATFORM(const struct drm_i915_private *i915,
> >> + enum intel_platform p, unsigned int s)
> >> +{
> >> + return __IS_SUBPLATFORM(i915, p, s);
> >> +}
> >>
> >> #define IS_I830(dev_priv) IS_PLATFORM(dev_priv, INTEL_I830)
> >> #define IS_I845G(dev_priv) IS_PLATFORM(dev_priv, INTEL_I845G)
> >> @@ -2296,11 +2335,15 @@ static inline unsigned int i915_sg_segment_size(void)
> >> #define IS_G45(dev_priv) IS_PLATFORM(dev_priv, INTEL_G45)
> >> #define IS_GM45(dev_priv) IS_PLATFORM(dev_priv, INTEL_GM45)
> >> #define IS_G4X(dev_priv) (IS_G45(dev_priv) || IS_GM45(dev_priv))
> >> -#define IS_PINEVIEW_G(dev_priv) (INTEL_DEVID(dev_priv) == 0xa001)
> >> -#define IS_PINEVIEW_M(dev_priv) (INTEL_DEVID(dev_priv) == 0xa011)
> >> +#define IS_PINEVIEW_G(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_PINEVIEW, INTEL_SUBPLATFORM_PINEVIEW_G)
> >> +#define IS_PINEVIEW_M(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_PINEVIEW, INTEL_SUBPLATFORM_PINEVIEW_M)
> >> #define IS_PINEVIEW(dev_priv) IS_PLATFORM(dev_priv, INTEL_PINEVIEW)
> >
> > I have a feeling we should just use the normal IS_MOBILE() thing
> > here. But untangling that mess might be a bit of work.
> >
> >> #define IS_G33(dev_priv) IS_PLATFORM(dev_priv, INTEL_G33)
> >> -#define IS_IRONLAKE_M(dev_priv) (INTEL_DEVID(dev_priv) == 0x0046)
> >> +#define IS_IRONLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_IRONLAKE)
> >> +#define IS_IRONLAKE_M(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_IRONLAKE, INTEL_SUBPLATFORM_IRONLAKE_M)
> >
> > This is clearly just IS_MOBILE().
>
> Ok.
>
> > Or we just keep the current pci id checks. Frankly I see no benefit in
> > abstracting these few exceptions.
>
> Are you referring to Ironlake only or in general?
pnv and ilk.
>
> >
> >> #define IS_IVYBRIDGE(dev_priv) IS_PLATFORM(dev_priv, INTEL_IVYBRIDGE)
> >> #define IS_IVB_GT1(dev_priv) (IS_IVYBRIDGE(dev_priv) && \
> >> INTEL_INFO(dev_priv)->gt == 1)
> >> @@ -2318,42 +2361,31 @@ static inline unsigned int i915_sg_segment_size(void)
> >> #define IS_MOBILE(dev_priv) (INTEL_INFO(dev_priv)->is_mobile)
> >> #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
> >> (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)
> >> -#define IS_BDW_ULT(dev_priv) (IS_BROADWELL(dev_priv) && \
> >> - ((INTEL_DEVID(dev_priv) & 0xf) == 0x6 || \
> >> - (INTEL_DEVID(dev_priv) & 0xf) == 0xb || \
> >> - (INTEL_DEVID(dev_priv) & 0xf) == 0xe))
> >> -/* ULX machines are also considered ULT. */
> >> -#define IS_BDW_ULX(dev_priv) (IS_BROADWELL(dev_priv) && \
> >> - (INTEL_DEVID(dev_priv) & 0xf) == 0xe)
> >> +#define IS_BDW_ULT(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_BROADWELL, INTEL_SUBPLATFORM_ULT)
> >> +#define IS_BDW_ULX(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_BROADWELL, INTEL_SUBPLATFORM_ULX)
> >> #define IS_BDW_GT3(dev_priv) (IS_BROADWELL(dev_priv) && \
> >> INTEL_INFO(dev_priv)->gt == 3)
> >> -#define IS_HSW_ULT(dev_priv) (IS_HASWELL(dev_priv) && \
> >> - (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0A00)
> >> +#define IS_HSW_ULT(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_HASWELL, INTEL_SUBPLATFORM_ULT)
> >> #define IS_HSW_GT3(dev_priv) (IS_HASWELL(dev_priv) && \
> >> INTEL_INFO(dev_priv)->gt == 3)
> >> #define IS_HSW_GT1(dev_priv) (IS_HASWELL(dev_priv) && \
> >> INTEL_INFO(dev_priv)->gt == 1)
> >> /* ULX machines are also considered ULT. */
> >> -#define IS_HSW_ULX(dev_priv) (INTEL_DEVID(dev_priv) == 0x0A0E || \
> >> - INTEL_DEVID(dev_priv) == 0x0A1E)
> >> -#define IS_SKL_ULT(dev_priv) (INTEL_DEVID(dev_priv) == 0x1906 || \
> >> - INTEL_DEVID(dev_priv) == 0x1913 || \
> >> - INTEL_DEVID(dev_priv) == 0x1916 || \
> >> - INTEL_DEVID(dev_priv) == 0x1921 || \
> >> - INTEL_DEVID(dev_priv) == 0x1926)
> >> -#define IS_SKL_ULX(dev_priv) (INTEL_DEVID(dev_priv) == 0x190E || \
> >> - INTEL_DEVID(dev_priv) == 0x1915 || \
> >> - INTEL_DEVID(dev_priv) == 0x191E)
> >> -#define IS_KBL_ULT(dev_priv) (INTEL_DEVID(dev_priv) == 0x5906 || \
> >> - INTEL_DEVID(dev_priv) == 0x5913 || \
> >> - INTEL_DEVID(dev_priv) == 0x5916 || \
> >> - INTEL_DEVID(dev_priv) == 0x5921 || \
> >> - INTEL_DEVID(dev_priv) == 0x5926)
> >> -#define IS_KBL_ULX(dev_priv) (INTEL_DEVID(dev_priv) == 0x590E || \
> >> - INTEL_DEVID(dev_priv) == 0x5915 || \
> >> - INTEL_DEVID(dev_priv) == 0x591E)
> >> -#define IS_AML_ULX(dev_priv) (INTEL_DEVID(dev_priv) == 0x591C || \
> >> - INTEL_DEVID(dev_priv) == 0x87C0)
> >> +#define IS_HSW_ULX(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_HASWELL, INTEL_SUBPLATFORM_ULX)
> >> +#define IS_SKL_ULT(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_SKYLAKE, INTEL_SUBPLATFORM_ULT)
> >> +#define IS_SKL_ULX(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_SKYLAKE, INTEL_SUBPLATFORM_ULX)
> >> +#define IS_KBL_ULT(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_KABYLAKE, INTEL_SUBPLATFORM_ULT)
> >> +#define IS_KBL_ULX(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_KABYLAKE, INTEL_SUBPLATFORM_ULX)
> >> +#define IS_AML_ULX(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_KABYLAKE, INTEL_SUBPLATFORM_AML_ULX)
> >
> > Why is AML_ULX different from normal ULX?
>
> As far as I could it is a different devid which we officially call
> Amberlake, but did not add it as separate platform.
>
> Given how this macro is used, which is always in conjuction with
> IS_KBL_ULX, I considered just removing it and adding the devid to
> KBL_ULX, but did not want to interferre with other people's ideas.
This whole AML_ULX makes no sense. The "non-ULX" AML is also
docucemented to be a Y SKU, so AFAICS it should use the exact
same codepaths as KBL Y (aka. KBL_ULX). Cc:ing Jose who added this
second AML variant...
I'm really tempted to just go and nuke all CFL,AML,etc. from the
code to make it actually match the spec.
>
> >
> >> #define IS_SKL_GT2(dev_priv) (IS_SKYLAKE(dev_priv) && \
> >> INTEL_INFO(dev_priv)->gt == 2)
> >> #define IS_SKL_GT3(dev_priv) (IS_SKYLAKE(dev_priv) && \
> >> @@ -2364,16 +2396,16 @@ static inline unsigned int i915_sg_segment_size(void)
> >> INTEL_INFO(dev_priv)->gt == 2)
> >> #define IS_KBL_GT3(dev_priv) (IS_KABYLAKE(dev_priv) && \
> >> INTEL_INFO(dev_priv)->gt == 3)
> >> -#define IS_CFL_ULT(dev_priv) (IS_COFFEELAKE(dev_priv) && \
> >> - (INTEL_DEVID(dev_priv) & 0x00F0) == 0x00A0)
> >> +#define IS_CFL_ULT(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_COFFEELAKE, INTEL_SUBPLATFORM_ULT)
> >> #define IS_CFL_GT2(dev_priv) (IS_COFFEELAKE(dev_priv) && \
> >> INTEL_INFO(dev_priv)->gt == 2)
> >> #define IS_CFL_GT3(dev_priv) (IS_COFFEELAKE(dev_priv) && \
> >> INTEL_INFO(dev_priv)->gt == 3)
> >> -#define IS_CNL_WITH_PORT_F(dev_priv) (IS_CANNONLAKE(dev_priv) && \
> >> - (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> >> -#define IS_ICL_WITH_PORT_F(dev_priv) (IS_ICELAKE(dev_priv) && \
> >> - INTEL_DEVID(dev_priv) != 0x8A51)
> >> +#define IS_CNL_WITH_PORT_F(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_CANNONLAKE, INTEL_SUBPLATFORM_PORTF)
> >> +#define IS_ICL_WITH_PORT_F(dev_priv) \
> >> + IS_SUBPLATFORM(dev_priv, INTEL_ICELAKE, INTEL_SUBPLATFORM_PORTF)
> >
> > God that PORTF thing is ugly. I would suggest we just add port_mask
> > or something along those lines into the device info.
> >
> > Then we could perhaps even avoid adding more and more branches to
> > intel_setup_outputs(). Although I'm not sure we need anything for that
> > since we'll look at the VBT anyway (which *maybe* doesn't advertize
> > ports that aren't present?).
>
> No idea, sounds like a question for you display folks to decide
> separately of this proposal probably?
>
> >
> >>
> >> #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> >> index 3cf697e8f1fa..2b042618e3ca 100644
> >> --- a/drivers/gpu/drm/i915/i915_pci.c
> >> +++ b/drivers/gpu/drm/i915/i915_pci.c
> >> @@ -32,7 +32,7 @@
> >> #include "i915_globals.h"
> >> #include "i915_selftest.h"
> >>
> >> -#define PLATFORM(x) .platform = (x), .platform_mask = BIT(x)
> >> +#define PLATFORM(x) .platform = (x)
> >> #define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1)
> >>
> >> #define I845_PIPE_OFFSETS \
> >> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> >> index aac19b1c419c..b8a7e17cb443 100644
> >> --- a/drivers/gpu/drm/i915/intel_device_info.c
> >> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> >> @@ -707,6 +707,85 @@ static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> >> return 0;
> >> }
> >>
> >> +void intel_device_info_subplatform_init(struct drm_i915_private *i915)
> >> +{
> >> + const unsigned int pbits =
> >> + BITS_PER_TYPE(RUNTIME_INFO(i915)->platform_mask[0]) -
> >> + INTEL_SUBPLATFORM_BITS;
> >> + const unsigned int pi = INTEL_INFO(i915)->platform / pbits;
> >> + const unsigned int pb =
> >> + INTEL_INFO(i915)->platform % pbits + INTEL_SUBPLATFORM_BITS;
> >> + struct intel_runtime_info *info = RUNTIME_INFO(i915);
> >> + u16 devid = INTEL_DEVID(i915);
> >> +
> >> + BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
> >> + pbits * ARRAY_SIZE(RUNTIME_INFO(i915)->platform_mask));
> >> +
> >> + info->platform_mask[pi] = BIT(pb);
> >> +
> >> + if (IS_PINEVIEW(i915)) {
> >> + if (devid == 0xa001)
> >> + info->platform_mask[pi] |=
> >> + BIT(INTEL_SUBPLATFORM_PINEVIEW_G);
> >> + else if (devid == 0xa011)
> >> + info->platform_mask[pi] |=
> >> + BIT(INTEL_SUBPLATFORM_PINEVIEW_M);
> >> + } else if (IS_IRONLAKE(i915)) {
> >> + if (devid == 0x0046)
> >> + info->platform_mask[pi] |=
> >> + BIT(INTEL_SUBPLATFORM_IRONLAKE_M);
> >> + } else if (IS_HASWELL(i915)) {
> >> + if ((devid & 0xFF00) == 0x0A00)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
> >> + /* ULX machines are also considered ULT. */
> >> + if (devid == 0x0A0E || devid == 0x0A1E)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULX);
> >> + } else if (IS_BROADWELL(i915)) {
> >> + if ((devid & 0xf) == 0x6 ||
> >> + (devid & 0xf) == 0xb ||
> >> + (devid & 0xf) == 0xe)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
> >> + /* ULX machines are also considered ULT. */
> >> + if ((devid & 0xf) == 0xe)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULX);
> >> + } else if (IS_SKYLAKE(i915)) {
> >> + if (devid == 0x1906 ||
> >> + devid == 0x1913 ||
> >> + devid == 0x1916 ||
> >> + devid == 0x1921 ||
> >> + devid == 0x1926)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
> >> + else if (devid == 0x190E ||
> >> + devid == 0x1915 ||
> >> + devid == 0x191E)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULX);
> >> + } else if (IS_KABYLAKE(i915)) {
> >> + if (devid == 0x5906 ||
> >> + devid == 0x5913 ||
> >> + devid == 0x5916 ||
> >> + devid == 0x5921 ||
> >> + devid == 0x5926)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
> >> + else if (devid == 0x590E ||
> >> + devid == 0x5915 ||
> >> + devid == 0x591E)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULX);
> >> + else if (devid == 0x591C ||
> >> + devid == 0x87C0)
> >> + info->platform_mask[pi] |=
> >> + BIT(INTEL_SUBPLATFORM_AML_ULX);
> >> + } else if (IS_COFFEELAKE(i915)) {
> >> + if ((devid & 0x00F0) == 0x00A0)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_ULT);
> >> + } else if (IS_CANNONLAKE(i915)) {
> >> + if ((devid & 0x0004) == 0x0004)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_PORTF);
> >> + } else if (IS_ICELAKE(i915)) {
> >> + if (devid != 0x8A51)
> >> + info->platform_mask[pi] |= BIT(INTEL_SUBPLATFORM_PORTF);
> >> + }
> >> +}
> >> +
> >> /**
> >> * intel_device_info_runtime_init - initialize runtime info
> >> * @dev_priv: the i915 device
> >> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> >> index 047d10bdd455..b03fbd2e451a 100644
> >> --- a/drivers/gpu/drm/i915/intel_device_info.h
> >> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> >> @@ -44,7 +44,7 @@ enum intel_platform {
> >> INTEL_I915G,
> >> INTEL_I915GM,
> >> INTEL_I945G,
> >> - INTEL_I945GM,
> >> + INTEL_I945GM = 8,
> >> INTEL_G33,
> >> INTEL_PINEVIEW,
> >> /* gen4 */
> >> @@ -55,7 +55,7 @@ enum intel_platform {
> >> /* gen5 */
> >> INTEL_IRONLAKE,
> >> /* gen6 */
> >> - INTEL_SANDYBRIDGE,
> >> + INTEL_SANDYBRIDGE = 16,
> >> /* gen7 */
> >> INTEL_IVYBRIDGE,
> >> INTEL_VALLEYVIEW,
> >> @@ -66,7 +66,7 @@ enum intel_platform {
> >> /* gen9 */
> >> INTEL_SKYLAKE,
> >> INTEL_BROXTON,
> >> - INTEL_KABYLAKE,
> >> + INTEL_KABYLAKE = 24,
> >
> > Too much magic. Looks rather fragile.
>
> Imagine these hunks gone. This is not required for this patch at all, or
> for anything for that matter. What do you think is magic and fragile?
Who is going to remeber to keep adding those magic numbers
in the future.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
