Re: [Intel-gfx] [PATCH v4 2/3] drm/i915/display: Convert intel_fb_modifier_to_tiling as non-static

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

On Tue, Nov 21, 2023 at 07:35:03AM +0000, Hogander, Jouni wrote:
> On Mon, 2023-11-20 at 16:19 +0200, Ville Syrjälä wrote:
> > On Mon, Nov 20, 2023 at 12:08:32PM +0200, Jouni Högander wrote:
> > > We are about to split i915 specific code from intel_fb.c. Convert
> > > intel_fb_modifier_to_tiling as non-static to allow calling it from
> > > split
> > > code.
> > 
> > We should only need this in the i915 path.
> 
> If I move it into i915 specific source file I need to convert 
> lookup_modifier as non-static. Would you prefer that or do you have
> some other suggestion?

Why does this even use lookup_modifier()? Hmm, I guess it's just
using that to reduce all the compressed modifiers to the Y-tiled
"class".

One alternative would be to just list the modifiers by hand, but
dunno if that's any better. I suppose we can just go with your
original idea.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> BR,
> 
> Jouni Högander
> > 
> > > 
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_fb.c | 40 ++++++++++++---------
> > > ----
> > >  drivers/gpu/drm/i915/display/intel_fb.h |  2 ++
> > >  2 files changed, 22 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
> > > b/drivers/gpu/drm/i915/display/intel_fb.c
> > > index 7c2df6c1f377..a235ec0f192d 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_fb.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> > > @@ -301,6 +301,26 @@ lookup_format_info(const struct
> > > drm_format_info formats[],
> > >         return NULL;
> > >  }
> > >  
> > > +unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
> > > +{
> > > +       u8 tiling_caps = lookup_modifier(fb_modifier)->plane_caps &
> > > +                        INTEL_PLANE_CAP_TILING_MASK;
> > > +
> > > +       switch (tiling_caps) {
> > > +       case INTEL_PLANE_CAP_TILING_Y:
> > > +               return I915_TILING_Y;
> > > +       case INTEL_PLANE_CAP_TILING_X:
> > > +               return I915_TILING_X;
> > > +       case INTEL_PLANE_CAP_TILING_4:
> > > +       case INTEL_PLANE_CAP_TILING_Yf:
> > > +       case INTEL_PLANE_CAP_TILING_NONE:
> > > +               return I915_TILING_NONE;
> > > +       default:
> > > +               MISSING_CASE(tiling_caps);
> > > +               return I915_TILING_NONE;
> > > +       }
> > > +}
> > > +
> > >  /**
> > >   * intel_fb_get_format_info: Get a modifier specific format
> > > information
> > >   * @cmd: FB add command structure
> > > @@ -737,26 +757,6 @@ intel_fb_align_height(const struct
> > > drm_framebuffer *fb,
> > >         return ALIGN(height, tile_height);
> > >  }
> > >  
> > > -static unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier)
> > > -{
> > > -       u8 tiling_caps = lookup_modifier(fb_modifier)->plane_caps &
> > > -                        INTEL_PLANE_CAP_TILING_MASK;
> > > -
> > > -       switch (tiling_caps) {
> > > -       case INTEL_PLANE_CAP_TILING_Y:
> > > -               return I915_TILING_Y;
> > > -       case INTEL_PLANE_CAP_TILING_X:
> > > -               return I915_TILING_X;
> > > -       case INTEL_PLANE_CAP_TILING_4:
> > > -       case INTEL_PLANE_CAP_TILING_Yf:
> > > -       case INTEL_PLANE_CAP_TILING_NONE:
> > > -               return I915_TILING_NONE;
> > > -       default:
> > > -               MISSING_CASE(tiling_caps);
> > > -               return I915_TILING_NONE;
> > > -       }
> > > -}
> > > -
> > >  bool intel_fb_modifier_uses_dpt(struct drm_i915_private *i915, u64
> > > modifier)
> > >  {
> > >         return HAS_DPT(i915) && modifier != DRM_FORMAT_MOD_LINEAR;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_fb.h
> > > b/drivers/gpu/drm/i915/display/intel_fb.h
> > > index e85167d6bc34..23db6628f53e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_fb.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_fb.h
> > > @@ -95,4 +95,6 @@ intel_user_framebuffer_create(struct drm_device
> > > *dev,
> > >  bool intel_fb_modifier_uses_dpt(struct drm_i915_private *i915, u64
> > > modifier);
> > >  bool intel_fb_uses_dpt(const struct drm_framebuffer *fb);
> > >  
> > > +unsigned int intel_fb_modifier_to_tiling(u64 fb_modifier);
> > > +
> > >  #endif /* __INTEL_FB_H__ */
> > > -- 
> > > 2.34.1
> > 
> 

-- 
Ville Syrjälä
Intel