Re: [PATCH 03/22] drm/i915: Shrink per-platform watermark configuration

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 06, 2016 at 05:38:12PM +0100, Tvrtko Ursulin wrote:
> 
> On 05/10/2016 18:11, Ville Syrjälä wrote:
> > On Wed, Oct 05, 2016 at 01:33:30PM +0100, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Use types of more appropriate size in struct
> >> intel_watermark_params to save 512 bytes of .rodata.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_drv.h | 10 +++++-----
> >>   drivers/gpu/drm/i915/intel_pm.c  |  4 ++--
> >>   2 files changed, 7 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index c52b1d3a7ba0..59a73f8ca7af 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -799,11 +799,11 @@ struct intel_plane {
> >>   };
> >>   
> >>   struct intel_watermark_params {
> >> -	unsigned long fifo_size;
> >> -	unsigned long max_wm;
> >> -	unsigned long default_wm;
> >> -	unsigned long guard_size;
> >> -	unsigned long cacheline_size;
> >> +	u16 fifo_size;
> >> +	u16 max_wm;
> >> +	u8 default_wm;
> >> +	u8 guard_size;
> >> +	u8 cacheline_size;
> >>   };
> > This thing has been bugging me since forever. And yet I never sent out a
> > fix. We could probably shrink things furher by tossing out a bunch of
> > the data since it's not all that diverse. But this looks like a decent
> > first step.
> >
> > The other thing that bugs me about these is the defines for the actual
> > values. Makes it just that much harder to figure out what the actual
> > values are. I'm pretty sure I have a branch or two where I kill the
> > defines but naturally I can't find it right now. I'm too lazy to
> > double check the values now with this indirection in place, but your
> > choice of types does look reasonable, so
> >
> > Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Thanks.
> 
> Btw we can shave one more kilobyte by similarly shrinking "struct 
> video_levels" and "struct tv_mode". u8/u16 are enough to replace most of 
> the ints there.  If you want to ack that in principle I can include a 
> patch for that in the next round.

Sure, go for it.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx