[Intel-gfx] [PATCH 1/2] drm/i915: Add a small wrapper to check for CCS modifiers.

[Dhinakaran Pandiyan]

On Wed, 2018-08-22 at 12:48 +0300, Ville Syrjälä wrote:
> On Tue, Aug 21, 2018 at 06:50:52PM -0700, Dhinakaran Pandiyan wrote:
> > Code looks cleaner with modifiers hidden inside this wrapper.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++----------
> >  drivers/gpu/drm/i915/intel_display.h |  1 +
> >  drivers/gpu/drm/i915/intel_sprite.c  |  3 +--
> >  3 files changed, 13 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index ad0f0e5389d9..b98eab113330 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2474,6 +2474,12 @@ intel_get_format_info(const struct
> > drm_mode_fb_cmd2 *cmd)
> >  	}
> >  }
> >  
> > +bool is_ccs_modifier(const u64 modifier)
> 
> const is rather pointless here IMO.
I thought of it as harmless documentation, will remove and send a v2.

> 
> > +{
> > +	return modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> > +	       modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
> > +}
> 
> IIRC I had a similar thing in my gtt remapping series, though I
> didn't
> plug it into all the places that could have used it.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
Thanks for the reviews. We have to get your plane refactor series
merged soon, I'm hoping I can make some time to start reviewing it this
week.

-DK

> > +
> >  static int
> >  intel_fill_fb_info(struct drm_i915_private *dev_priv,
> >  		   struct drm_framebuffer *fb)
> > @@ -2504,8 +2510,7 @@ intel_fill_fb_info(struct drm_i915_private
> > *dev_priv,
> >  			return ret;
> >  		}
> >  
> > -		if ((fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS
> > ||
> > -		     fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS)
> > && i == 1) {
> > +		if (is_ccs_modifier(fb->modifier) && i == 1) {
> >  			int hsub = fb->format->hsub;
> >  			int vsub = fb->format->vsub;
> >  			int tile_width, tile_height;
> > @@ -3054,8 +3059,7 @@ static int skl_check_main_surface(const
> > struct intel_crtc_state *crtc_state,
> >  	 * CCS AUX surface doesn't have its own x/y offsets, we
> > must make sure
> >  	 * they match with the main surface x/y offsets.
> >  	 */
> > -	if (fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> > -	    fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS) {
> > +	if (is_ccs_modifier(fb->modifier)) {
> >  		while
> > (!skl_check_main_ccs_coordinates(plane_state, x, y, offset)) {
> >  			if (offset == 0)
> >  				break;
> > @@ -3189,8 +3193,7 @@ int skl_check_plane_surface(const struct
> > intel_crtc_state *crtc_state,
> >  		ret = skl_check_nv12_aux_surface(plane_state);
> >  		if (ret)
> >  			return ret;
> > -	} else if (fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> > -		   fb->modifier == I915_FORMAT_MOD_Yf_TILED_CCS) {
> > +	} else if (is_ccs_modifier(fb->modifier)) {
> >  		ret = skl_check_ccs_aux_surface(plane_state);
> >  		if (ret)
> >  			return ret;
> > @@ -13399,8 +13402,7 @@ static bool
> > skl_plane_format_mod_supported(struct drm_plane *_plane,
> >  	case DRM_FORMAT_XBGR8888:
> >  	case DRM_FORMAT_ARGB8888:
> >  	case DRM_FORMAT_ABGR8888:
> > -		if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
> > -		    modifier == I915_FORMAT_MOD_Y_TILED_CCS)
> > +		if (is_ccs_modifier(modifier))
> >  			return true;
> >  		/* fall through */
> >  	case DRM_FORMAT_RGB565:
> > @@ -14596,8 +14598,7 @@ static int intel_framebuffer_init(struct
> > intel_framebuffer *intel_fb,
> >  		 * potential runtime errors at plane configuration
> > time.
> >  		 */
> >  		if (IS_GEN9(dev_priv) && i == 0 && fb->width >
> > 3840 &&
> > -		    (fb->modifier == I915_FORMAT_MOD_Y_TILED_CCS
> > ||
> > -		     fb->modifier ==
> > I915_FORMAT_MOD_Yf_TILED_CCS))
> > +		    is_ccs_modifier(fb->modifier))
> >  			stride_alignment *= 4;
> >  
> >  		if (fb->pitches[i] & (stride_alignment - 1)) {
> > diff --git a/drivers/gpu/drm/i915/intel_display.h
> > b/drivers/gpu/drm/i915/intel_display.h
> > index a04c5a495a2b..dbd8b1acf9d6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.h
> > +++ b/drivers/gpu/drm/i915/intel_display.h
> > @@ -381,4 +381,5 @@ void intel_link_compute_m_n(int bpp, int
> > nlanes,
> >  			    struct intel_link_m_n *m_n,
> >  			    bool reduce_m_n);
> >  
> > +bool is_ccs_modifier(const u64 modifier);
> >  #endif
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index f7026e887fa9..b49adcd93892 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1407,8 +1407,7 @@ static bool
> > skl_plane_format_mod_supported(struct drm_plane *_plane,
> >  	case DRM_FORMAT_XBGR8888:
> >  	case DRM_FORMAT_ARGB8888:
> >  	case DRM_FORMAT_ABGR8888:
> > -		if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
> > -		    modifier == I915_FORMAT_MOD_Y_TILED_CCS)
> > +		if (is_ccs_modifier(modifier))
> >  			return true;
> >  		/* fall through */
> >  	case DRM_FORMAT_RGB565:
> > -- 
> > 2.17.1
> 
>