Re: [igt-dev] [PATCH i-g-t v3 12/21] lib/igt_vc4: Add helper for checking SAND tiling support on plane

Maxime Ripard
2019-01-15T15:57:01Z

On Mon, Jan 14, 2019 at 04:01:05PM +0100, Paul Kocialkowski wrote:
> Hi,
>
> On Fri, 2019-01-11 at 16:15 +0100, Maxime Ripard wrote:
> > On Fri, Jan 11, 2019 at 10:05:23AM +0100, Paul Kocialkowski wrote:
> > > This introduces a convenience helper for checking whether a plane
> > > supports SAND tiling for a given format.
> > >
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > ---
> > > lib/igt_vc4.h | 24 ++++++++++++++++++++++++
> > > 1 file changed, 24 insertions(+)
> > >
> > > diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> > > index 9118ae193f60..cb980541a61f 100644
> > > --- a/lib/igt_vc4.h
> > > +++ b/lib/igt_vc4.h
> > > @@ -33,6 +33,30 @@ static inline bool igt_vc4_plane_supports_t_tiling(igt_plane_t *plane,
> > > DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED);
> > > }
> > >
> > > +static inline bool igt_vc4_plane_supports_sand_tiling(igt_plane_t *plane,
> > > + uint32_t format,
> > > + size_t column_width_bytes)
> > > +{
> > > + uint64_t modifier;
> > > +
> > > + switch (column_width_bytes) {
> > > + case 32:
> > > + modifier = DRM_FORMAT_MOD_BROADCOM_SAND32;
> > > + break;
> > > + case 64:
> > > + modifier = DRM_FORMAT_MOD_BROADCOM_SAND64;
> > > + break;
> > > + case 128:
> > > + modifier = DRM_FORMAT_MOD_BROADCOM_SAND128;
> > > + break;
> > > + case 256:
> > > + modifier = DRM_FORMAT_MOD_BROADCOM_SAND256;
> > > + break;
> > > + }
> > > +
> > > + return igt_plane_has_format_mod(plane, format, modifier);
> > > +}
> > > +
> >
> > I'm not quite sure what that function is supposed to be doing, or
> > rather, why do you need the column_width_bytes parameter.
> >
> > If that function was just about checking whether the sand tiling is
> > supported, then you can just return an or'd igt_plane_has_format_mod
> > of all the modifiers supported, right?
>
> Well, some variants of SAND are only valid for a reduced set of
> formats. IIRC, SAND256 only applies to NV12 while others apply to other
> YUV formats as well.
>
> So I believe it's relevant to distinguish between the different SAND
> tilings when checking for support.

Ok

> However, the function could be simplified by simply passing the
> modifier, extracting the base modifier (without column height, that is
> coded in the modifier) and calling igt_plane_has_format_mod.

Yeah, that sounds much better.

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering