Hello,
Not much to be added, everything is in the subject line. The first patch prepares for the modification by refactoring code, and the second patch adds the check.
The patches are based on a merge of drm-next and drm-fixes.
Laurent Pinchart (2): drm: Share plane pixel format check code between legacy and atomic drm: Check in setcrtc if the primary plane supports the fb pixel format
drivers/gpu/drm/drm_atomic.c | 10 ++++------ drivers/gpu/drm/drm_crtc.c | 41 +++++++++++++++++++++++++++++++++++------ include/drm/drm_crtc.h | 2 ++ 3 files changed, 41 insertions(+), 12 deletions(-)
Both the legacy and atomic helpers need to check whether a plane supports a given pixel format. The code is currently duplicated, share it.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/drm_atomic.c | 10 ++++------ drivers/gpu/drm/drm_crtc.c | 25 +++++++++++++++++++------ include/drm/drm_crtc.h | 2 ++ 3 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 321e098..eb4d874 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -473,7 +473,7 @@ static int drm_atomic_plane_check(struct drm_plane *plane, struct drm_plane_state *state) { unsigned int fb_width, fb_height; - unsigned int i; + int ret;
/* either *both* CRTC and FB must be set, or neither */ if (WARN_ON(state->crtc && !state->fb)) { @@ -495,13 +495,11 @@ static int drm_atomic_plane_check(struct drm_plane *plane, }
/* Check whether this plane supports the fb pixel format. */ - for (i = 0; i < plane->format_count; i++) - if (state->fb->pixel_format == plane->format_types[i]) - break; - if (i == plane->format_count) { + ret = drm_plane_check_pixel_format(plane, state->fb->pixel_format); + if (ret) { DRM_DEBUG_ATOMIC("Invalid pixel format %s\n", drm_get_format_name(state->fb->pixel_format)); - return -EINVAL; + return ret; }
/* Give drivers some help against integer overflows */ diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 927f344..d794bcf 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2421,6 +2421,23 @@ int drm_mode_getplane(struct drm_device *dev, void *data, return 0; }
+/** + * drm_plane_check_pixel_format - Check if the plane supports the pixel format + * @plane: plane to check for format support + * @format: the pixel format + */ +int drm_plane_check_pixel_format(const struct drm_plane *plane, u32 format) +{ + unsigned int i; + + for (i = 0; i < plane->format_count; i++) { + if (format == plane->format_types[i]) + return 0; + } + + return -EINVAL; +} + /* * setplane_internal - setplane handler for internal callers * @@ -2441,7 +2458,6 @@ static int __setplane_internal(struct drm_plane *plane, { int ret = 0; unsigned int fb_width, fb_height; - unsigned int i;
/* No fb means shut it down */ if (!fb) { @@ -2464,13 +2480,10 @@ static int __setplane_internal(struct drm_plane *plane, }
/* Check whether this plane supports the fb pixel format. */ - for (i = 0; i < plane->format_count; i++) - if (fb->pixel_format == plane->format_types[i]) - break; - if (i == plane->format_count) { + ret = drm_plane_check_pixel_format(plane, fb->pixel_format); + if (ret) { DRM_DEBUG_KMS("Invalid pixel format %s\n", drm_get_format_name(fb->pixel_format)); - ret = -EINVAL; goto out; }
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b1465d6..da83d39 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1263,6 +1263,8 @@ extern int drm_plane_init(struct drm_device *dev, extern void drm_plane_cleanup(struct drm_plane *plane); extern unsigned int drm_plane_index(struct drm_plane *plane); extern void drm_plane_force_disable(struct drm_plane *plane); +extern int drm_plane_check_pixel_format(const struct drm_plane *plane, + u32 format); extern void drm_crtc_get_hv_timing(const struct drm_display_mode *mode, int *hdisplay, int *vdisplay); extern int drm_crtc_check_viewport(const struct drm_crtc *crtc,
Drivers implementing the universal planes API report the list of supported pixel formats for the primary plane. Make sure the fb passed to the setcrtc ioctl is compatible.
Drivers not implementing the universal planes API will have no format reported for the primary plane, skip the check in that case.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/drm_crtc.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d794bcf..09ac312 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2807,6 +2807,22 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
+ /* + * Check whether the primary plane supports the fb pixel format. + * Drivers not implementing the universal planes API will have + * no format reported for the primary plane, skip the check in + * that case. + */ + if (crtc->primary->format_count) { + ret = drm_plane_check_pixel_format(crtc->primary, + fb->pixel_format); + if (ret) { + DRM_DEBUG_KMS("Invalid pixel format %s\n", + drm_get_format_name(fb->pixel_format)); + goto out; + } + } + ret = drm_crtc_check_viewport(crtc, crtc_req->x, crtc_req->y, mode, fb); if (ret)
On Thu, Mar 05, 2015 at 02:25:44AM +0200, Laurent Pinchart wrote:
Drivers implementing the universal planes API report the list of supported pixel formats for the primary plane. Make sure the fb passed to the setcrtc ioctl is compatible.
Drivers not implementing the universal planes API will have no format reported for the primary plane, skip the check in that case.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/gpu/drm/drm_crtc.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d794bcf..09ac312 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2807,6 +2807,22 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
/** Check whether the primary plane supports the fb pixel format.* Drivers not implementing the universal planes API will have* no format reported for the primary plane, skip the check in* that case.*/if (crtc->primary->format_count) {
The compat helpers create a primary plane for you, and they fill out the format list with 2 hopefully save ones. I think we need to add some boolean to drm_plane which drm_primary_helper_create_plane sets when no explicit format list is passed in.
And I guess we should make drm_primary_helper_create_plane static while at it.
I've pulled in your first patch into drm-misc, with a slightly extended docbook text.
Thanks, Daniel
ret = drm_plane_check_pixel_format(crtc->primary,fb->pixel_format);if (ret) {DRM_DEBUG_KMS("Invalid pixel format %s\n",drm_get_format_name(fb->pixel_format));goto out;}}- ret = drm_crtc_check_viewport(crtc, crtc_req->x, crtc_req->y, mode, fb); if (ret)
-- 2.0.5
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Laurent Pinchart