From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/2] drm/i915: Agnostic INTEL_INFO Date: Mon, 11 Aug 2014 13:25:49 +0200 Message-ID: <20140811112549.GK8727@phenom.ffwll.local> References: <1407608323-30693-1-git-send-email-chris@chris-wilson.co.uk> <1407608323-30693-2-git-send-email-chris@chris-wilson.co.uk> <87zjfb5o4l.fsf@intel.com> <20140811102607.GG6666@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f46.google.com (mail-wg0-f46.google.com [74.125.82.46]) by gabe.freedesktop.org (Postfix) with ESMTP id 2D3AB6E3BA for ; Mon, 11 Aug 2014 04:25:38 -0700 (PDT) Received: by mail-wg0-f46.google.com with SMTP id m15so8433077wgh.5 for ; Mon, 11 Aug 2014 04:25:37 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140811102607.GG6666@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Jani Nikula , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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 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 > > > --- > > > 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