| Message ID | 20180822015053.1420-1-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915: Add a small wrapper to check for CCS modifiers. | expand |
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@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. > +{ > + 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@linux.intel.com> > + > 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
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@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@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 > >
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) +{ + return modifier == I915_FORMAT_MOD_Y_TILED_CCS || + modifier == I915_FORMAT_MOD_Yf_TILED_CCS; +} + 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:
Code looks cleaner with modifiers hidden inside this wrapper. Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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(-)