On Thu, Mar 15, 2018 at 10:42:17AM -0700, Eric Anholt wrote: > Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> writes: > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > To make life easier for drivers, let's have the core check that the > > requested pixel format is supported by at least one plane when creating > > a new framebuffer. > > > > This eases the burden on drivers by making sure they'll never get > > requests to create fbs with unsupported pixel formats. Thanks to the > > new .fb_modifier() hook this check can now be done whether the request > > specifies the modifier directly or driver has to deduce it from the > > gem bo tiling (or via any other method really). > > > > v0: Accept anyformat if the driver doesn't do planes (Eric) > > s/planes_have_format/any_plane_has_format/ (Eric) > > Check the modifier as well since we already have a function > > that does both > > v3: Don't do the check in the core since we may not know the > > modifier yet, instead export the function and let drivers > > call it themselves > > v4: Unexport the functiona and put the format_default check back > > since this will again be called by the core, ie. undo v3 ;) > > > > Cc: Eric Anholt <eric@xxxxxxxxxx> > > Testcase: igt/kms_addfb_basic/expected-formats > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/drm_framebuffer.c | 30 ++++++++++++++++++++++++++++++ > > 1 file changed, 30 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > > index 21d3d51eb261..e618a6b728d4 100644 > > --- a/drivers/gpu/drm/drm_framebuffer.c > > +++ b/drivers/gpu/drm/drm_framebuffer.c > > @@ -152,6 +152,26 @@ static int fb_plane_height(int height, > > return DIV_ROUND_UP(height, format->vsub); > > } > > > > +static bool any_plane_has_format(struct drm_device *dev, > > + u32 format, u64 modifier) > > +{ > > + struct drm_plane *plane; > > + > > + drm_for_each_plane(plane, dev) { > > + /* > > + * In case the driver doesn't really do > > + * planes we have to accept any format here. > > + */ > > + if (plane->format_default) > > + return true; > > + > > + if (drm_plane_check_pixel_format(plane, format, modifier) == 0) > > + return true; > > + } > > + > > + return false; > > +} > > This drm_plane_check_pixel_format() will always fail for VC4's SAND > modifiers or VC5's UIF modifiers, where we're using the middle 48 bits > as a bit of metadata (like how we have horizontal stride passed outside > of the modifier) and you can't list all of the possible values in an > array on the plane. Hmm. drm_atomic_plane_check() etc. call this thing as well. How does that manage to work currently? -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel