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

Paul Kocialkowski
2019-01-14T15:01:17Z

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.

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.

Cheers,

Paul

--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering