Re: [RFC] Prefer INTEL_INFO(dev_priv) to INTEL_INFO(dev)

[Dave Gordon <david.s.gordon@intel.com>]

On 08/04/16 07:09, Joonas Lahtinen wrote:
> On to, 2016-04-07 at 18:57 +0100, Dave Gordon wrote:
>> Where we have a suitable dev_priv pointer, we can use that rather than
>> 'dev' for accessing INTEL_INFO().  This removes one level of memory
>> reference, decreasing code size a little and possibly making everything
>> a little faster. We could also do this for all the macros that implicitly
>> use INTEL_INFO e.g. IS_CHERRYVIEW().
>
> If applied to all macros, sounds like the right thing to do. Although,
> I would prefer to see s/dev_priv/i915/g and then i915->info.gen, i915-
>> info.is_cherryview etc. rather than dozens of macros.

Maybe, but not all callsites actually have a 'dev_priv' in scope at 
present, which is why this had to be done with Coccinelle rather than 
just sed.

I tried extending this to all IS_XXX() macros as well (but not as yet 
HAS_XXX()) and got:

  36 files changed, 741 insertions(+), 763 deletions(-)

the line count difference being due to 22 now-unused local variables 
deleted afterwards; the i915.ko binary shrank by about 2k - which 
represents quite a lot of instructions. So this isn't just a matter of 
style, like changing INTEL_INFO(dev)->gen with INTEL_GEN(dev), this 
transformation actually improves code size and presumably speed :)

> It's going to cause a lot of rebasing, though. So depending on when we
> apply dev_priv -> i915, might be or might not be worth the hassle now.
>
> Regards, Joonas

The point of including the actual Cocci code in the commit message is 
that then anyone who has a set of not-yet-submitted changes can apply 
the Cocci script to their own codebase before attempting to rebase onto 
the new version of nightly, thus eliminating from the conflicts all 
those which are simply due to the patch applied to upstream.

For this reason, I'd like to recommend that anyone doing this sort of 
bulk transformation with Cocci or awk or just sed should /always/ 
include the transformation script to help those with patches in flight.

.Dave.

>> Coccinelle:
>>
>>      @dev_priv_param@
>>        function FUNC;
>>        idexpression struct drm_device *DEV;
>>        identifier DEV_PRIV;
>>      @@
>>        FUNC(..., struct drm_i915_private *DEV_PRIV, ...)
>>        {
>> 	<...
>> 	INTEL_INFO
>> 	(
>>      -     DEV
>>      +     DEV_PRIV
>> 	)
>> 	...>
>>        }
>>
>>      @dev_priv_local@
>>        idexpression struct drm_device *DEV;
>>        identifier DEV_PRIV;
>>        expression E;
>>      @@
>>        {
>> 	...
>>      (
>> 	struct drm_i915_private *DEV_PRIV;
>>      |
>> 	struct drm_i915_private *DEV_PRIV = E;
>>      )
>> 	<...
>> 	INTEL_INFO
>> 	(
>>      -     DEV
>>      +     DEV_PRIV
>> 	)
>> 	...>
>>        }
>>
>> followed by manually deleting 6 now-unused "dev" locals.
>>
>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c        |  66 +++++++-------
>>   drivers/gpu/drm/i915/i915_dma.c            |  30 +++---
>>   drivers/gpu/drm/i915/i915_drv.c            |   4 +-
>>   drivers/gpu/drm/i915/i915_gem.c            |   6 +-
>>   drivers/gpu/drm/i915/i915_gem_context.c    |   2 +-
>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |   8 +-
>>   drivers/gpu/drm/i915/i915_gem_fence.c      |   8 +-
>>   drivers/gpu/drm/i915/i915_gem_gtt.c        |  14 +--
>>   drivers/gpu/drm/i915/i915_gem_stolen.c     |   6 +-
>>   drivers/gpu/drm/i915/i915_gpu_error.c      |  32 +++----
>>   drivers/gpu/drm/i915/i915_irq.c            |  36 ++++----
>>   drivers/gpu/drm/i915/i915_suspend.c        |  20 ++--
>>   drivers/gpu/drm/i915/intel_color.c         |  16 ++--
>>   drivers/gpu/drm/i915/intel_crt.c           |   6 +-
>>   drivers/gpu/drm/i915/intel_ddi.c           |   4 +-
>>   drivers/gpu/drm/i915/intel_display.c       | 141 ++++++++++++++---------------
>>   drivers/gpu/drm/i915/intel_dp.c            |  20 ++--
>>   drivers/gpu/drm/i915/intel_dpll_mgr.c      |   2 +-
>>   drivers/gpu/drm/i915/intel_fbdev.c         |   4 +-
>>   drivers/gpu/drm/i915/intel_lrc.c           |   2 +-
>>   drivers/gpu/drm/i915/intel_lvds.c          |   4 +-
>>   drivers/gpu/drm/i915/intel_overlay.c       |   4 +-
>>   drivers/gpu/drm/i915/intel_pm.c            |  50 +++++-----
>>   drivers/gpu/drm/i915/intel_psr.c           |   6 +-
>>   drivers/gpu/drm/i915/intel_ringbuffer.c    |  40 ++++----
>>   drivers/gpu/drm/i915/intel_sdvo.c          |   8 +-
>>   drivers/gpu/drm/i915/intel_tv.c            |   2 +-
>>   drivers/gpu/drm/i915/intel_uncore.c        |   6 +-
>>   28 files changed, 270 insertions(+), 277 deletions(-)

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