* [PATCH 1/2] drm/i915: Copy PCI device id into the device info block @ 2014-08-09 18:18 Chris Wilson 2014-08-09 18:18 ` [PATCH 2/2] drm/i915: Agnostic INTEL_INFO Chris Wilson 2014-08-11 10:05 ` [PATCH 1/2] drm/i915: Copy PCI device id into the device info block Jani Nikula 0 siblings, 2 replies; 8+ messages in thread From: Chris Wilson @ 2014-08-09 18:18 UTC (permalink / raw) To: intel-gfx This is so that we can make the drm_i915_private->info always the preferred source for chipset type and feature queries. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_dma.c | 5 +++-- drivers/gpu/drm/i915/i915_drv.h | 50 +++++++++++++++++++++-------------------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 96bf11e7a011..39988940d468 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1592,9 +1592,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) dev->dev_private = dev_priv; dev_priv->dev = dev; - /* copy initial configuration to dev_priv->info */ + /* Setup the write-once "constant" device info */ device_info = (struct intel_device_info *)&dev_priv->info; - *device_info = *info; + memcpy(device_info, info, sizeof(dev_priv->info)); + device_info->device_id = dev->pdev->device; spin_lock_init(&dev_priv->irq_lock); spin_lock_init(&dev_priv->gpu_error.lock); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b3d42b085790..0aef763ffa75 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -563,6 +563,7 @@ struct intel_uncore { struct intel_device_info { u32 display_mmio_offset; + u16 device_id; u8 num_pipes:3; u8 num_sprites[I915_MAX_PIPES]; u8 gen; @@ -2062,51 +2063,52 @@ struct drm_i915_cmd_table { int count; }; -#define INTEL_INFO(dev) (&to_i915(dev)->info) +#define INTEL_INFO(p) (&to_i915(p)->info) +#define INTEL_DEVID(p) (INTEL_INFO(p)->device_id) -#define IS_I830(dev) ((dev)->pdev->device == 0x3577) -#define IS_845G(dev) ((dev)->pdev->device == 0x2562) +#define IS_I830(dev) (INTEL_DEVID(dev) == 0x3577) +#define IS_845G(dev) (INTEL_DEVID(dev) == 0x2562) #define IS_I85X(dev) (INTEL_INFO(dev)->is_i85x) -#define IS_I865G(dev) ((dev)->pdev->device == 0x2572) +#define IS_I865G(dev) (INTEL_DEVID(dev) == 0x2572) #define IS_I915G(dev) (INTEL_INFO(dev)->is_i915g) -#define IS_I915GM(dev) ((dev)->pdev->device == 0x2592) -#define IS_I945G(dev) ((dev)->pdev->device == 0x2772) +#define IS_I915GM(dev) (INTEL_DEVID(dev) == 0x2592) +#define IS_I945G(dev) (INTEL_DEVID(dev) == 0x2772) #define IS_I945GM(dev) (INTEL_INFO(dev)->is_i945gm) #define IS_BROADWATER(dev) (INTEL_INFO(dev)->is_broadwater) #define IS_CRESTLINE(dev) (INTEL_INFO(dev)->is_crestline) -#define IS_GM45(dev) ((dev)->pdev->device == 0x2A42) +#define IS_GM45(dev) (INTEL_DEVID(dev) == 0x2A42) #define IS_G4X(dev) (INTEL_INFO(dev)->is_g4x) -#define IS_PINEVIEW_G(dev) ((dev)->pdev->device == 0xa001) -#define IS_PINEVIEW_M(dev) ((dev)->pdev->device == 0xa011) +#define IS_PINEVIEW_G(dev) (INTEL_DEVID(dev) == 0xa001) +#define IS_PINEVIEW_M(dev) (INTEL_DEVID(dev) == 0xa011) #define IS_PINEVIEW(dev) (INTEL_INFO(dev)->is_pineview) #define IS_G33(dev) (INTEL_INFO(dev)->is_g33) -#define IS_IRONLAKE_M(dev) ((dev)->pdev->device == 0x0046) +#define IS_IRONLAKE_M(dev) (INTEL_DEVID(dev) == 0x0046) #define IS_IVYBRIDGE(dev) (INTEL_INFO(dev)->is_ivybridge) -#define IS_IVB_GT1(dev) ((dev)->pdev->device == 0x0156 || \ - (dev)->pdev->device == 0x0152 || \ - (dev)->pdev->device == 0x015a) -#define IS_SNB_GT1(dev) ((dev)->pdev->device == 0x0102 || \ - (dev)->pdev->device == 0x0106 || \ - (dev)->pdev->device == 0x010A) +#define IS_IVB_GT1(dev) (INTEL_DEVID(dev) == 0x0156 || \ + INTEL_DEVID(dev) == 0x0152 || \ + INTEL_DEVID(dev) == 0x015a) +#define IS_SNB_GT1(dev) (INTEL_DEVID(dev) == 0x0102 || \ + INTEL_DEVID(dev) == 0x0106 || \ + INTEL_DEVID(dev) == 0x010A) #define IS_VALLEYVIEW(dev) (INTEL_INFO(dev)->is_valleyview) #define IS_CHERRYVIEW(dev) (INTEL_INFO(dev)->is_valleyview && IS_GEN8(dev)) #define IS_HASWELL(dev) (INTEL_INFO(dev)->is_haswell) #define IS_BROADWELL(dev) (!INTEL_INFO(dev)->is_valleyview && IS_GEN8(dev)) #define IS_MOBILE(dev) (INTEL_INFO(dev)->is_mobile) #define IS_HSW_EARLY_SDV(dev) (IS_HASWELL(dev) && \ - ((dev)->pdev->device & 0xFF00) == 0x0C00) + (INTEL_DEVID(dev) & 0xFF00) == 0x0C00) #define IS_BDW_ULT(dev) (IS_BROADWELL(dev) && \ - (((dev)->pdev->device & 0xf) == 0x2 || \ - ((dev)->pdev->device & 0xf) == 0x6 || \ - ((dev)->pdev->device & 0xf) == 0xe)) + ((INTEL_DEVID(dev) & 0xf) == 0x2 || \ + (INTEL_DEVID(dev) & 0xf) == 0x6 || \ + (INTEL_DEVID(dev) & 0xf) == 0xe)) #define IS_HSW_ULT(dev) (IS_HASWELL(dev) && \ - ((dev)->pdev->device & 0xFF00) == 0x0A00) + (INTEL_DEVID(dev) & 0xFF00) == 0x0A00) #define IS_ULT(dev) (IS_HSW_ULT(dev) || IS_BDW_ULT(dev)) #define IS_HSW_GT3(dev) (IS_HASWELL(dev) && \ - ((dev)->pdev->device & 0x00F0) == 0x0020) + (INTEL_DEVID(dev) & 0x00F0) == 0x0020) /* ULX machines are also considered ULT. */ -#define IS_HSW_ULX(dev) ((dev)->pdev->device == 0x0A0E || \ - (dev)->pdev->device == 0x0A1E) +#define IS_HSW_ULX(dev) (INTEL_DEVID(dev) == 0x0A0E || \ + INTEL_DEVID(dev) == 0x0A1E) #define IS_PRELIMINARY_HW(intel_info) ((intel_info)->is_preliminary) /* -- 2.1.0.rc1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] drm/i915: Agnostic INTEL_INFO 2014-08-09 18:18 [PATCH 1/2] drm/i915: Copy PCI device id into the device info block Chris Wilson @ 2014-08-09 18:18 ` Chris Wilson 2014-08-11 10:14 ` Jani Nikula 2014-08-11 10:05 ` [PATCH 1/2] drm/i915: Copy PCI device id into the device info block Jani Nikula 1 sibling, 1 reply; 8+ messages in thread From: Chris Wilson @ 2014-08-09 18:18 UTC (permalink / raw) To: intel-gfx Adapt the macro so that we can pass either the struct drm_device or the struct drm_i915_private pointers and get the answer we want. Over time, my plan is to convert all users over to using drm_i915_private and so trimming down the pointer dance. Having spent a few hours chasing that goal and achieved over 8k of object code saving, it appears to be a worthwhile target. This interim macro allows us to slowly convert over. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_dma.c | 3 +++ drivers/gpu/drm/i915/i915_drv.h | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 39988940d468..49149406fcd8 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1585,6 +1585,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) if (!drm_core_check_feature(dev, DRIVER_MODESET) && !dev->agp) return -EINVAL; + /* For the ugly agnostic INTEL_INFO macro */ + BUILD_BUG_ON(sizeof(*dev_priv) == sizeof(*dev)); + dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); if (dev_priv == NULL) return -ENOMEM; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 0aef763ffa75..e66465430bdc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2063,7 +2063,8 @@ struct drm_i915_cmd_table { int count; }; -#define INTEL_INFO(p) (&to_i915(p)->info) +#define __I915__(p) ((sizeof(*(p)) == sizeof(struct drm_i915_private)) ? (struct drm_i915_private *)(p) : to_i915((struct drm_device *)p)) +#define INTEL_INFO(p) (&__I915__(p)->info) #define INTEL_DEVID(p) (INTEL_INFO(p)->device_id) #define IS_I830(dev) (INTEL_DEVID(dev) == 0x3577) -- 2.1.0.rc1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915: Agnostic INTEL_INFO 2014-08-09 18:18 ` [PATCH 2/2] drm/i915: Agnostic INTEL_INFO Chris Wilson @ 2014-08-11 10:14 ` Jani Nikula 2014-08-11 10:26 ` Chris Wilson 0 siblings, 1 reply; 8+ messages in thread From: Jani Nikula @ 2014-08-11 10:14 UTC (permalink / raw) To: Chris Wilson, intel-gfx On Sat, 09 Aug 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Adapt the macro so that we can pass either the struct drm_device or the > struct drm_i915_private pointers and get the answer we want. Over time, > my plan is to convert all users over to using drm_i915_private and so > trimming down the pointer dance. Having spent a few hours chasing that > goal and achieved over 8k of object code saving, it appears to be a > worthwhile target. This interim macro allows us to slowly convert over. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_dma.c | 3 +++ > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 39988940d468..49149406fcd8 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1585,6 +1585,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > if (!drm_core_check_feature(dev, DRIVER_MODESET) && !dev->agp) > return -EINVAL; > > + /* For the ugly agnostic INTEL_INFO macro */ > + BUILD_BUG_ON(sizeof(*dev_priv) == sizeof(*dev)); > + > dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); > if (dev_priv == NULL) > return -ENOMEM; > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 0aef763ffa75..e66465430bdc 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2063,7 +2063,8 @@ struct drm_i915_cmd_table { > int count; > }; > > -#define INTEL_INFO(p) (&to_i915(p)->info) > +#define __I915__(p) ((sizeof(*(p)) == sizeof(struct drm_i915_private)) ? (struct drm_i915_private *)(p) : to_i915((struct drm_device *)p)) The explicit casts make me uncomfortable. Indeed, aren't they completely unnecessary? If the sizeof matches, p is expected to be struct drm_i915_private *, and if not, it's expected to be struct drm_device *, right? #define __I915__(p) ((sizeof(*(p)) == sizeof(struct drm_i915_private)) ? (p) : to_i915(p)) Otherwise you won't even get a compiler warning if you pass a random pointer to this macro. BR, Jani. > +#define INTEL_INFO(p) (&__I915__(p)->info) > #define INTEL_DEVID(p) (INTEL_INFO(p)->device_id) > > #define IS_I830(dev) (INTEL_DEVID(dev) == 0x3577) > -- > 2.1.0.rc1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915: Agnostic INTEL_INFO 2014-08-11 10:14 ` Jani Nikula @ 2014-08-11 10:26 ` Chris Wilson 2014-08-11 11:25 ` Daniel Vetter 0 siblings, 1 reply; 8+ messages in thread From: Chris Wilson @ 2014-08-11 10:26 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Mon, Aug 11, 2014 at 01:14:50PM +0300, Jani Nikula wrote: > On Sat, 09 Aug 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Adapt the macro so that we can pass either the struct drm_device or the > > struct drm_i915_private pointers and get the answer we want. Over time, > > my plan is to convert all users over to using drm_i915_private and so > > trimming down the pointer dance. Having spent a few hours chasing that > > goal and achieved over 8k of object code saving, it appears to be a > > worthwhile target. This interim macro allows us to slowly convert over. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_dma.c | 3 +++ > > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > index 39988940d468..49149406fcd8 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -1585,6 +1585,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > > if (!drm_core_check_feature(dev, DRIVER_MODESET) && !dev->agp) > > return -EINVAL; > > > > + /* For the ugly agnostic INTEL_INFO macro */ > > + BUILD_BUG_ON(sizeof(*dev_priv) == sizeof(*dev)); > > + > > dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); > > if (dev_priv == NULL) > > return -ENOMEM; > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 0aef763ffa75..e66465430bdc 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2063,7 +2063,8 @@ struct drm_i915_cmd_table { > > int count; > > }; > > > > -#define INTEL_INFO(p) (&to_i915(p)->info) > > +#define __I915__(p) ((sizeof(*(p)) == sizeof(struct drm_i915_private)) ? (struct drm_i915_private *)(p) : to_i915((struct drm_device *)p)) > > The explicit casts make me uncomfortable. Indeed, aren't they completely > unnecessary? If the sizeof matches, p is expected to be struct > drm_i915_private *, and if not, it's expected to be struct drm_device *, > right? Yes, killing the typesafety is bad. Sadly it is to quiesce the compiler as the type-mismatch warnings are generated before it does the dead code elimination removing the constant ?: expression. Too bad we couldn't simply write typeof(*p) == typeof(T). -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915: Agnostic INTEL_INFO 2014-08-11 10:26 ` Chris Wilson @ 2014-08-11 11:25 ` Daniel Vetter 2014-08-11 11:30 ` Chris Wilson 0 siblings, 1 reply; 8+ messages in thread From: Daniel Vetter @ 2014-08-11 11:25 UTC (permalink / raw) To: Chris Wilson, Jani Nikula, intel-gfx On Mon, Aug 11, 2014 at 11:26:07AM +0100, Chris Wilson wrote: > On Mon, Aug 11, 2014 at 01:14:50PM +0300, Jani Nikula wrote: > > On Sat, 09 Aug 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > Adapt the macro so that we can pass either the struct drm_device or the > > > struct drm_i915_private pointers and get the answer we want. Over time, > > > my plan is to convert all users over to using drm_i915_private and so > > > trimming down the pointer dance. Having spent a few hours chasing that > > > goal and achieved over 8k of object code saving, it appears to be a > > > worthwhile target. This interim macro allows us to slowly convert over. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > --- > > > drivers/gpu/drm/i915/i915_dma.c | 3 +++ > > > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > > index 39988940d468..49149406fcd8 100644 > > > --- a/drivers/gpu/drm/i915/i915_dma.c > > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > > @@ -1585,6 +1585,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > > > if (!drm_core_check_feature(dev, DRIVER_MODESET) && !dev->agp) > > > return -EINVAL; > > > > > > + /* For the ugly agnostic INTEL_INFO macro */ > > > + BUILD_BUG_ON(sizeof(*dev_priv) == sizeof(*dev)); > > > + > > > dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); > > > if (dev_priv == NULL) > > > return -ENOMEM; > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index 0aef763ffa75..e66465430bdc 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -2063,7 +2063,8 @@ struct drm_i915_cmd_table { > > > int count; > > > }; > > > > > > -#define INTEL_INFO(p) (&to_i915(p)->info) > > > +#define __I915__(p) ((sizeof(*(p)) == sizeof(struct drm_i915_private)) ? (struct drm_i915_private *)(p) : to_i915((struct drm_device *)p)) > > > > The explicit casts make me uncomfortable. Indeed, aren't they completely > > unnecessary? If the sizeof matches, p is expected to be struct > > drm_i915_private *, and if not, it's expected to be struct drm_device *, > > right? > > Yes, killing the typesafety is bad. Sadly it is to quiesce the compiler > as the type-mismatch warnings are generated before it does the dead code > elimination removing the constant ?: expression. Too bad we couldn't > simply write typeof(*p) == typeof(T). Do we need the 2nd cast for (struct drm_device *)? If we could drop that we'd have about as much type-safety as before, presuming no one manages to matche our drm_i915_private exactly in size. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915: Agnostic INTEL_INFO 2014-08-11 11:25 ` Daniel Vetter @ 2014-08-11 11:30 ` Chris Wilson 0 siblings, 0 replies; 8+ messages in thread From: Chris Wilson @ 2014-08-11 11:30 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Mon, Aug 11, 2014 at 01:25:49PM +0200, Daniel Vetter wrote: > On Mon, Aug 11, 2014 at 11:26:07AM +0100, Chris Wilson wrote: > > On Mon, Aug 11, 2014 at 01:14:50PM +0300, Jani Nikula wrote: > > > On Sat, 09 Aug 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > Adapt the macro so that we can pass either the struct drm_device or the > > > > struct drm_i915_private pointers and get the answer we want. Over time, > > > > my plan is to convert all users over to using drm_i915_private and so > > > > trimming down the pointer dance. Having spent a few hours chasing that > > > > goal and achieved over 8k of object code saving, it appears to be a > > > > worthwhile target. This interim macro allows us to slowly convert over. > > > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > --- > > > > drivers/gpu/drm/i915/i915_dma.c | 3 +++ > > > > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > > > index 39988940d468..49149406fcd8 100644 > > > > --- a/drivers/gpu/drm/i915/i915_dma.c > > > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > > > @@ -1585,6 +1585,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > > > > if (!drm_core_check_feature(dev, DRIVER_MODESET) && !dev->agp) > > > > return -EINVAL; > > > > > > > > + /* For the ugly agnostic INTEL_INFO macro */ > > > > + BUILD_BUG_ON(sizeof(*dev_priv) == sizeof(*dev)); > > > > + > > > > dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); > > > > if (dev_priv == NULL) > > > > return -ENOMEM; > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > > index 0aef763ffa75..e66465430bdc 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > > @@ -2063,7 +2063,8 @@ struct drm_i915_cmd_table { > > > > int count; > > > > }; > > > > > > > > -#define INTEL_INFO(p) (&to_i915(p)->info) > > > > +#define __I915__(p) ((sizeof(*(p)) == sizeof(struct drm_i915_private)) ? (struct drm_i915_private *)(p) : to_i915((struct drm_device *)p)) > > > > > > The explicit casts make me uncomfortable. Indeed, aren't they completely > > > unnecessary? If the sizeof matches, p is expected to be struct > > > drm_i915_private *, and if not, it's expected to be struct drm_device *, > > > right? > > > > Yes, killing the typesafety is bad. Sadly it is to quiesce the compiler > > as the type-mismatch warnings are generated before it does the dead code > > elimination removing the constant ?: expression. Too bad we couldn't > > simply write typeof(*p) == typeof(T). > > Do we need the 2nd cast for (struct drm_device *)? If we could drop that > we'd have about as much type-safety as before, presuming no one manages to > matche our drm_i915_private exactly in size. The compiler is happy with that. I can't remember exactly why I added it first time, it certainly wasn't for its looks. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/i915: Copy PCI device id into the device info block 2014-08-09 18:18 [PATCH 1/2] drm/i915: Copy PCI device id into the device info block Chris Wilson 2014-08-09 18:18 ` [PATCH 2/2] drm/i915: Agnostic INTEL_INFO Chris Wilson @ 2014-08-11 10:05 ` Jani Nikula 2014-08-11 11:26 ` Daniel Vetter 1 sibling, 1 reply; 8+ messages in thread From: Jani Nikula @ 2014-08-11 10:05 UTC (permalink / raw) To: Chris Wilson, intel-gfx On Sat, 09 Aug 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote: > This is so that we can make the drm_i915_private->info always the > preferred source for chipset type and feature queries. Reviewed-by: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_dma.c | 5 +++-- > drivers/gpu/drm/i915/i915_drv.h | 50 +++++++++++++++++++++-------------------- > 2 files changed, 29 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 96bf11e7a011..39988940d468 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1592,9 +1592,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > dev->dev_private = dev_priv; > dev_priv->dev = dev; > > - /* copy initial configuration to dev_priv->info */ > + /* Setup the write-once "constant" device info */ > device_info = (struct intel_device_info *)&dev_priv->info; > - *device_info = *info; > + memcpy(device_info, info, sizeof(dev_priv->info)); > + device_info->device_id = dev->pdev->device; > > spin_lock_init(&dev_priv->irq_lock); > spin_lock_init(&dev_priv->gpu_error.lock); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index b3d42b085790..0aef763ffa75 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -563,6 +563,7 @@ struct intel_uncore { > > struct intel_device_info { > u32 display_mmio_offset; > + u16 device_id; > u8 num_pipes:3; > u8 num_sprites[I915_MAX_PIPES]; > u8 gen; > @@ -2062,51 +2063,52 @@ struct drm_i915_cmd_table { > int count; > }; > > -#define INTEL_INFO(dev) (&to_i915(dev)->info) > +#define INTEL_INFO(p) (&to_i915(p)->info) > +#define INTEL_DEVID(p) (INTEL_INFO(p)->device_id) > > -#define IS_I830(dev) ((dev)->pdev->device == 0x3577) > -#define IS_845G(dev) ((dev)->pdev->device == 0x2562) > +#define IS_I830(dev) (INTEL_DEVID(dev) == 0x3577) > +#define IS_845G(dev) (INTEL_DEVID(dev) == 0x2562) > #define IS_I85X(dev) (INTEL_INFO(dev)->is_i85x) > -#define IS_I865G(dev) ((dev)->pdev->device == 0x2572) > +#define IS_I865G(dev) (INTEL_DEVID(dev) == 0x2572) > #define IS_I915G(dev) (INTEL_INFO(dev)->is_i915g) > -#define IS_I915GM(dev) ((dev)->pdev->device == 0x2592) > -#define IS_I945G(dev) ((dev)->pdev->device == 0x2772) > +#define IS_I915GM(dev) (INTEL_DEVID(dev) == 0x2592) > +#define IS_I945G(dev) (INTEL_DEVID(dev) == 0x2772) > #define IS_I945GM(dev) (INTEL_INFO(dev)->is_i945gm) > #define IS_BROADWATER(dev) (INTEL_INFO(dev)->is_broadwater) > #define IS_CRESTLINE(dev) (INTEL_INFO(dev)->is_crestline) > -#define IS_GM45(dev) ((dev)->pdev->device == 0x2A42) > +#define IS_GM45(dev) (INTEL_DEVID(dev) == 0x2A42) > #define IS_G4X(dev) (INTEL_INFO(dev)->is_g4x) > -#define IS_PINEVIEW_G(dev) ((dev)->pdev->device == 0xa001) > -#define IS_PINEVIEW_M(dev) ((dev)->pdev->device == 0xa011) > +#define IS_PINEVIEW_G(dev) (INTEL_DEVID(dev) == 0xa001) > +#define IS_PINEVIEW_M(dev) (INTEL_DEVID(dev) == 0xa011) > #define IS_PINEVIEW(dev) (INTEL_INFO(dev)->is_pineview) > #define IS_G33(dev) (INTEL_INFO(dev)->is_g33) > -#define IS_IRONLAKE_M(dev) ((dev)->pdev->device == 0x0046) > +#define IS_IRONLAKE_M(dev) (INTEL_DEVID(dev) == 0x0046) > #define IS_IVYBRIDGE(dev) (INTEL_INFO(dev)->is_ivybridge) > -#define IS_IVB_GT1(dev) ((dev)->pdev->device == 0x0156 || \ > - (dev)->pdev->device == 0x0152 || \ > - (dev)->pdev->device == 0x015a) > -#define IS_SNB_GT1(dev) ((dev)->pdev->device == 0x0102 || \ > - (dev)->pdev->device == 0x0106 || \ > - (dev)->pdev->device == 0x010A) > +#define IS_IVB_GT1(dev) (INTEL_DEVID(dev) == 0x0156 || \ > + INTEL_DEVID(dev) == 0x0152 || \ > + INTEL_DEVID(dev) == 0x015a) > +#define IS_SNB_GT1(dev) (INTEL_DEVID(dev) == 0x0102 || \ > + INTEL_DEVID(dev) == 0x0106 || \ > + INTEL_DEVID(dev) == 0x010A) > #define IS_VALLEYVIEW(dev) (INTEL_INFO(dev)->is_valleyview) > #define IS_CHERRYVIEW(dev) (INTEL_INFO(dev)->is_valleyview && IS_GEN8(dev)) > #define IS_HASWELL(dev) (INTEL_INFO(dev)->is_haswell) > #define IS_BROADWELL(dev) (!INTEL_INFO(dev)->is_valleyview && IS_GEN8(dev)) > #define IS_MOBILE(dev) (INTEL_INFO(dev)->is_mobile) > #define IS_HSW_EARLY_SDV(dev) (IS_HASWELL(dev) && \ > - ((dev)->pdev->device & 0xFF00) == 0x0C00) > + (INTEL_DEVID(dev) & 0xFF00) == 0x0C00) > #define IS_BDW_ULT(dev) (IS_BROADWELL(dev) && \ > - (((dev)->pdev->device & 0xf) == 0x2 || \ > - ((dev)->pdev->device & 0xf) == 0x6 || \ > - ((dev)->pdev->device & 0xf) == 0xe)) > + ((INTEL_DEVID(dev) & 0xf) == 0x2 || \ > + (INTEL_DEVID(dev) & 0xf) == 0x6 || \ > + (INTEL_DEVID(dev) & 0xf) == 0xe)) > #define IS_HSW_ULT(dev) (IS_HASWELL(dev) && \ > - ((dev)->pdev->device & 0xFF00) == 0x0A00) > + (INTEL_DEVID(dev) & 0xFF00) == 0x0A00) > #define IS_ULT(dev) (IS_HSW_ULT(dev) || IS_BDW_ULT(dev)) > #define IS_HSW_GT3(dev) (IS_HASWELL(dev) && \ > - ((dev)->pdev->device & 0x00F0) == 0x0020) > + (INTEL_DEVID(dev) & 0x00F0) == 0x0020) > /* ULX machines are also considered ULT. */ > -#define IS_HSW_ULX(dev) ((dev)->pdev->device == 0x0A0E || \ > - (dev)->pdev->device == 0x0A1E) > +#define IS_HSW_ULX(dev) (INTEL_DEVID(dev) == 0x0A0E || \ > + INTEL_DEVID(dev) == 0x0A1E) > #define IS_PRELIMINARY_HW(intel_info) ((intel_info)->is_preliminary) > > /* > -- > 2.1.0.rc1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/i915: Copy PCI device id into the device info block 2014-08-11 10:05 ` [PATCH 1/2] drm/i915: Copy PCI device id into the device info block Jani Nikula @ 2014-08-11 11:26 ` Daniel Vetter 0 siblings, 0 replies; 8+ messages in thread From: Daniel Vetter @ 2014-08-11 11:26 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Mon, Aug 11, 2014 at 01:05:03PM +0300, Jani Nikula wrote: > On Sat, 09 Aug 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > This is so that we can make the drm_i915_private->info always the > > preferred source for chipset type and feature queries. > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> Queued for -next, thanks for the patch. -Daniel > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_dma.c | 5 +++-- > > drivers/gpu/drm/i915/i915_drv.h | 50 +++++++++++++++++++++-------------------- > > 2 files changed, 29 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > index 96bf11e7a011..39988940d468 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -1592,9 +1592,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > > dev->dev_private = dev_priv; > > dev_priv->dev = dev; > > > > - /* copy initial configuration to dev_priv->info */ > > + /* Setup the write-once "constant" device info */ > > device_info = (struct intel_device_info *)&dev_priv->info; > > - *device_info = *info; > > + memcpy(device_info, info, sizeof(dev_priv->info)); > > + device_info->device_id = dev->pdev->device; > > > > spin_lock_init(&dev_priv->irq_lock); > > spin_lock_init(&dev_priv->gpu_error.lock); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index b3d42b085790..0aef763ffa75 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -563,6 +563,7 @@ struct intel_uncore { > > > > struct intel_device_info { > > u32 display_mmio_offset; > > + u16 device_id; > > u8 num_pipes:3; > > u8 num_sprites[I915_MAX_PIPES]; > > u8 gen; > > @@ -2062,51 +2063,52 @@ struct drm_i915_cmd_table { > > int count; > > }; > > > > -#define INTEL_INFO(dev) (&to_i915(dev)->info) > > +#define INTEL_INFO(p) (&to_i915(p)->info) > > +#define INTEL_DEVID(p) (INTEL_INFO(p)->device_id) > > > > -#define IS_I830(dev) ((dev)->pdev->device == 0x3577) > > -#define IS_845G(dev) ((dev)->pdev->device == 0x2562) > > +#define IS_I830(dev) (INTEL_DEVID(dev) == 0x3577) > > +#define IS_845G(dev) (INTEL_DEVID(dev) == 0x2562) > > #define IS_I85X(dev) (INTEL_INFO(dev)->is_i85x) > > -#define IS_I865G(dev) ((dev)->pdev->device == 0x2572) > > +#define IS_I865G(dev) (INTEL_DEVID(dev) == 0x2572) > > #define IS_I915G(dev) (INTEL_INFO(dev)->is_i915g) > > -#define IS_I915GM(dev) ((dev)->pdev->device == 0x2592) > > -#define IS_I945G(dev) ((dev)->pdev->device == 0x2772) > > +#define IS_I915GM(dev) (INTEL_DEVID(dev) == 0x2592) > > +#define IS_I945G(dev) (INTEL_DEVID(dev) == 0x2772) > > #define IS_I945GM(dev) (INTEL_INFO(dev)->is_i945gm) > > #define IS_BROADWATER(dev) (INTEL_INFO(dev)->is_broadwater) > > #define IS_CRESTLINE(dev) (INTEL_INFO(dev)->is_crestline) > > -#define IS_GM45(dev) ((dev)->pdev->device == 0x2A42) > > +#define IS_GM45(dev) (INTEL_DEVID(dev) == 0x2A42) > > #define IS_G4X(dev) (INTEL_INFO(dev)->is_g4x) > > -#define IS_PINEVIEW_G(dev) ((dev)->pdev->device == 0xa001) > > -#define IS_PINEVIEW_M(dev) ((dev)->pdev->device == 0xa011) > > +#define IS_PINEVIEW_G(dev) (INTEL_DEVID(dev) == 0xa001) > > +#define IS_PINEVIEW_M(dev) (INTEL_DEVID(dev) == 0xa011) > > #define IS_PINEVIEW(dev) (INTEL_INFO(dev)->is_pineview) > > #define IS_G33(dev) (INTEL_INFO(dev)->is_g33) > > -#define IS_IRONLAKE_M(dev) ((dev)->pdev->device == 0x0046) > > +#define IS_IRONLAKE_M(dev) (INTEL_DEVID(dev) == 0x0046) > > #define IS_IVYBRIDGE(dev) (INTEL_INFO(dev)->is_ivybridge) > > -#define IS_IVB_GT1(dev) ((dev)->pdev->device == 0x0156 || \ > > - (dev)->pdev->device == 0x0152 || \ > > - (dev)->pdev->device == 0x015a) > > -#define IS_SNB_GT1(dev) ((dev)->pdev->device == 0x0102 || \ > > - (dev)->pdev->device == 0x0106 || \ > > - (dev)->pdev->device == 0x010A) > > +#define IS_IVB_GT1(dev) (INTEL_DEVID(dev) == 0x0156 || \ > > + INTEL_DEVID(dev) == 0x0152 || \ > > + INTEL_DEVID(dev) == 0x015a) > > +#define IS_SNB_GT1(dev) (INTEL_DEVID(dev) == 0x0102 || \ > > + INTEL_DEVID(dev) == 0x0106 || \ > > + INTEL_DEVID(dev) == 0x010A) > > #define IS_VALLEYVIEW(dev) (INTEL_INFO(dev)->is_valleyview) > > #define IS_CHERRYVIEW(dev) (INTEL_INFO(dev)->is_valleyview && IS_GEN8(dev)) > > #define IS_HASWELL(dev) (INTEL_INFO(dev)->is_haswell) > > #define IS_BROADWELL(dev) (!INTEL_INFO(dev)->is_valleyview && IS_GEN8(dev)) > > #define IS_MOBILE(dev) (INTEL_INFO(dev)->is_mobile) > > #define IS_HSW_EARLY_SDV(dev) (IS_HASWELL(dev) && \ > > - ((dev)->pdev->device & 0xFF00) == 0x0C00) > > + (INTEL_DEVID(dev) & 0xFF00) == 0x0C00) > > #define IS_BDW_ULT(dev) (IS_BROADWELL(dev) && \ > > - (((dev)->pdev->device & 0xf) == 0x2 || \ > > - ((dev)->pdev->device & 0xf) == 0x6 || \ > > - ((dev)->pdev->device & 0xf) == 0xe)) > > + ((INTEL_DEVID(dev) & 0xf) == 0x2 || \ > > + (INTEL_DEVID(dev) & 0xf) == 0x6 || \ > > + (INTEL_DEVID(dev) & 0xf) == 0xe)) > > #define IS_HSW_ULT(dev) (IS_HASWELL(dev) && \ > > - ((dev)->pdev->device & 0xFF00) == 0x0A00) > > + (INTEL_DEVID(dev) & 0xFF00) == 0x0A00) > > #define IS_ULT(dev) (IS_HSW_ULT(dev) || IS_BDW_ULT(dev)) > > #define IS_HSW_GT3(dev) (IS_HASWELL(dev) && \ > > - ((dev)->pdev->device & 0x00F0) == 0x0020) > > + (INTEL_DEVID(dev) & 0x00F0) == 0x0020) > > /* ULX machines are also considered ULT. */ > > -#define IS_HSW_ULX(dev) ((dev)->pdev->device == 0x0A0E || \ > > - (dev)->pdev->device == 0x0A1E) > > +#define IS_HSW_ULX(dev) (INTEL_DEVID(dev) == 0x0A0E || \ > > + INTEL_DEVID(dev) == 0x0A1E) > > #define IS_PRELIMINARY_HW(intel_info) ((intel_info)->is_preliminary) > > > > /* > > -- > > 2.1.0.rc1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-08-11 11:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-08-09 18:18 [PATCH 1/2] drm/i915: Copy PCI device id into the device info block Chris Wilson 2014-08-09 18:18 ` [PATCH 2/2] drm/i915: Agnostic INTEL_INFO Chris Wilson 2014-08-11 10:14 ` Jani Nikula 2014-08-11 10:26 ` Chris Wilson 2014-08-11 11:25 ` Daniel Vetter 2014-08-11 11:30 ` Chris Wilson 2014-08-11 10:05 ` [PATCH 1/2] drm/i915: Copy PCI device id into the device info block Jani Nikula 2014-08-11 11:26 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox