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

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

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>

>  
>  struct cxsr_latency {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3b1f0b40ccb9..96d0c57c816c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -775,13 +775,13 @@ static bool g4x_check_srwm(struct drm_device *dev,
>  		      display_wm, cursor_wm);
>  
>  	if (display_wm > display->max_wm) {
> -		DRM_DEBUG_KMS("display watermark is too large(%d/%ld), disabling\n",
> +		DRM_DEBUG_KMS("display watermark is too large(%d/%u), disabling\n",
>  			      display_wm, display->max_wm);
>  		return false;
>  	}
>  
>  	if (cursor_wm > cursor->max_wm) {
> -		DRM_DEBUG_KMS("cursor watermark is too large(%d/%ld), disabling\n",
> +		DRM_DEBUG_KMS("cursor watermark is too large(%d/%u), disabling\n",
>  			      cursor_wm, cursor->max_wm);
>  		return false;
>  	}
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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