On Thu, Mar 15, 2018 at 10:42:17AM -0700, Eric Anholt wrote:
Ville Syrjala ville.syrjala@linux.intel.com writes:
From: Ville Syrjälä ville.syrjala@linux.intel.com
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@anholt.net Testcase: igt/kms_addfb_basic/expected-formats Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
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?