Re: [PATCH] drm/vc4: Check for valid formats

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

On Mon, Jan 02, 2023 at 03:20:06PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 02.01.23 um 14:57 schrieb Maíra Canal:
> > Currently, vc4 is not checking valid formats before creating a
> > framebuffer, which is triggering the IGT test
> > igt@kms_addfb_basic@addfb25-bad-modifier to fail, as vc4 accepts
> > to create a framebuffer with an invalid modifier. Therefore, check
> > for valid formats before creating framebuffers on vc4 and vc5 in
> > order to avoid creating framebuffers with invalid formats.
> > 
> > Signed-off-by: Maíra Canal <mcanal@igalia.com>
> > ---
> >   drivers/gpu/drm/vc4/vc4_kms.c | 23 ++++++++++++++++++++++-
> >   1 file changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> > index 53d9f30460cf..5d1afd66fcc1 100644
> > --- a/drivers/gpu/drm/vc4/vc4_kms.c
> > +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> > @@ -500,6 +500,27 @@ static struct drm_framebuffer *vc4_fb_create(struct drm_device *dev,
> > 		mode_cmd = &mode_cmd_local;
> > 	}
> > 
> > +	if (!drm_any_plane_has_format(dev, mode_cmd->pixel_format,
> > +				      mode_cmd->modifier[0])) {
> > +		drm_dbg_kms(dev, "Unsupported pixel format %p4cc / modifier 0x%llx\n",
> > +			    &mode_cmd->pixel_format, mode_cmd->modifier[0]);
> > +		return ERR_PTR(-EINVAL);
> > +	}
> 
> This might be a stupid question, but why doesn't drm_fbgem_fb_create() 
> do this test already? It seems like a no-brainer and *not* testing for 
> the plane formats should be the exception.

That was the approach I tried originally but there were a bunch of
problems with various drivers it at the time. Dunno if all of those
got sorted out or not. IIRC the idea floating around for ancient
drivers was to skip the check based on plane->format_default. Looks
like we're already using that approach in the setcrtc ioctl.

-- 
Ville Syrjälä
Intel