From: Ankit Nautiyal ankit.k.nautiyal@intel.com
This patch series is a re-attempt to enable aspect ratio support in DRM layer. Currently the aspect ratio information gets lost in translation during a user->kernel mode or vice versa.
The old patch series (https://pw-emeril.freedesktop.org/series/10850/) had 4 patches, out of which 2 patches were reverted due to lack of drm client protection while loading the aspect information.
This patch series also includes 5 patches from Ville Syrjälä's series for 'Info-frame cleanup and fixes': https://patchwork.freedesktop.org/series/33730/ which fixes the mode matching mechanism via flags, and also ensures that no bogus aspect-ratios are sent in the AVI infoframes.
This patch series, adds a DRM client option for aspect ratio, and loads aspect ratio flags, only when the client sets this cap.
To test this patch, the testdiplay IGT test is modified to have an option to do a modeset with only aspect ratio modes. Also, there is a userspace implementation in Wayland/weston layer: https://patchwork.freedesktop.org/patch/188125/ (Which is already ACK'ed by wayland community.)
This, helps us in passing HDMI compliance test cases like 7-27, where the test equipment applies a CEA mode, and expects the exact VIC in the AVI infoframes.
Ankit Nautiyal (4): drm: Add DRM client cap for aspect-ratio drm: Handle aspect-ratio info in getblob drm: Handle aspect ratio info in legacy and atomic modeset paths drm: Expose modes with aspect ratio, only if requested
Sharma, Shashank (2): drm: Add aspect ratio parsing in DRM layer drm: Add and handle new aspect ratios in DRM layer
Ville Syrjälä (5): drm/modes: Introduce drm_mode_match() drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy drm/edid: Fix cea mode aspect ratio handling drm/edid: Don't send bogus aspect ratios in AVI infoframes video/hdmi: Reject illegal picture aspect ratios
drivers/gpu/drm/drm_atomic.c | 36 ++++-- drivers/gpu/drm/drm_atomic_helper.c | 6 +- drivers/gpu/drm/drm_connector.c | 40 ++++++- drivers/gpu/drm/drm_crtc.c | 8 ++ drivers/gpu/drm/drm_crtc_internal.h | 3 +- drivers/gpu/drm/drm_edid.c | 41 +++++-- drivers/gpu/drm/drm_fb_helper.c | 12 +- drivers/gpu/drm/drm_ioctl.c | 5 + drivers/gpu/drm/drm_mode_object.c | 9 +- drivers/gpu/drm/drm_modes.c | 226 +++++++++++++++++++++++++++++++----- drivers/gpu/drm/drm_property.c | 5 + drivers/video/hdmi.c | 3 + include/drm/drm_atomic.h | 5 +- include/drm/drm_file.h | 8 ++ include/drm/drm_modes.h | 13 +++ include/drm/drm_property.h | 2 + include/uapi/drm/drm.h | 7 ++ include/uapi/drm/drm_mode.h | 6 + 18 files changed, 369 insertions(+), 66 deletions(-)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Make mode matching less confusing by allowing the caller to specify which parts of the modes should match via some flags.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/drm_modes.c | 134 ++++++++++++++++++++++++++++++++++---------- include/drm/drm_modes.h | 9 +++ 2 files changed, 112 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 5a8033f..a48672c 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -940,17 +940,68 @@ struct drm_display_mode *drm_mode_duplicate(struct drm_device *dev, } EXPORT_SYMBOL(drm_mode_duplicate);
+static bool drm_mode_match_timings(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return mode1->hdisplay == mode2->hdisplay && + mode1->hsync_start == mode2->hsync_start && + mode1->hsync_end == mode2->hsync_end && + mode1->htotal == mode2->htotal && + mode1->hskew == mode2->hskew && + mode1->vdisplay == mode2->vdisplay && + mode1->vsync_start == mode2->vsync_start && + mode1->vsync_end == mode2->vsync_end && + mode1->vtotal == mode2->vtotal && + mode1->vscan == mode2->vscan; +} + +static bool drm_mode_match_clock(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + /* + * do clock check convert to PICOS + * so fb modes get matched the same + */ + if (mode1->clock && mode2->clock) + return KHZ2PICOS(mode1->clock) == KHZ2PICOS(mode2->clock); + else + return mode1->clock == mode2->clock; +} + +static bool drm_mode_match_flags(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) == + (mode2->flags & ~DRM_MODE_FLAG_3D_MASK); +} + +static bool drm_mode_match_3d_flags(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return (mode1->flags & DRM_MODE_FLAG_3D_MASK) == + (mode2->flags & DRM_MODE_FLAG_3D_MASK); +} + +static bool drm_mode_match_aspect_ratio(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return mode1->picture_aspect_ratio == mode2->picture_aspect_ratio; +} + /** - * drm_mode_equal - test modes for equality + * drm_mode_match - test modes for (partial) equality * @mode1: first mode * @mode2: second mode + * @match_flags: which parts need to match (DRM_MODE_MATCH_*) * * Check to see if @mode1 and @mode2 are equivalent. * * Returns: - * True if the modes are equal, false otherwise. + * True if the modes are (partially) equal, false otherwise. */ -bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2) +bool drm_mode_match(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2, + unsigned int match_flags) { if (!mode1 && !mode2) return true; @@ -958,15 +1009,48 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ if (!mode1 || !mode2) return false;
- /* do clock check convert to PICOS so fb modes get matched - * the same */ - if (mode1->clock && mode2->clock) { - if (KHZ2PICOS(mode1->clock) != KHZ2PICOS(mode2->clock)) - return false; - } else if (mode1->clock != mode2->clock) + if (match_flags & DRM_MODE_MATCH_TIMINGS && + !drm_mode_match_timings(mode1, mode2)) + return false; + + if (match_flags & DRM_MODE_MATCH_CLOCK && + !drm_mode_match_clock(mode1, mode2)) + return false; + + if (match_flags & DRM_MODE_MATCH_FLAGS && + !drm_mode_match_flags(mode1, mode2)) + return false; + + if (match_flags & DRM_MODE_MATCH_3D_FLAGS && + !drm_mode_match_3d_flags(mode1, mode2)) return false;
- return drm_mode_equal_no_clocks(mode1, mode2); + if (match_flags & DRM_MODE_MATCH_ASPECT_RATIO && + !drm_mode_match_aspect_ratio(mode1, mode2)) + return false; + + return true; +} +EXPORT_SYMBOL(drm_mode_match); + +/** + * drm_mode_equal - test modes for equality + * @mode1: first mode + * @mode2: second mode + * + * Check to see if @mode1 and @mode2 are equivalent. + * + * Returns: + * True if the modes are equal, false otherwise. + */ +bool drm_mode_equal(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return drm_mode_match(mode1, mode2, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_CLOCK | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS); } EXPORT_SYMBOL(drm_mode_equal);
@@ -981,13 +1065,13 @@ EXPORT_SYMBOL(drm_mode_equal); * Returns: * True if the modes are equal, false otherwise. */ -bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2) +bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) { - if ((mode1->flags & DRM_MODE_FLAG_3D_MASK) != - (mode2->flags & DRM_MODE_FLAG_3D_MASK)) - return false; - - return drm_mode_equal_no_clocks_no_stereo(mode1, mode2); + return drm_mode_match(mode1, mode2, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS); } EXPORT_SYMBOL(drm_mode_equal_no_clocks);
@@ -1005,21 +1089,9 @@ EXPORT_SYMBOL(drm_mode_equal_no_clocks); bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2) { - if (mode1->hdisplay == mode2->hdisplay && - mode1->hsync_start == mode2->hsync_start && - mode1->hsync_end == mode2->hsync_end && - mode1->htotal == mode2->htotal && - mode1->hskew == mode2->hskew && - mode1->vdisplay == mode2->vdisplay && - mode1->vsync_start == mode2->vsync_start && - mode1->vsync_end == mode2->vsync_end && - mode1->vtotal == mode2->vtotal && - mode1->vscan == mode2->vscan && - (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) == - (mode2->flags & ~DRM_MODE_FLAG_3D_MASK)) - return true; - - return false; + return drm_mode_match(mode1, mode2, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_FLAGS); } EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 0d310be..2f78b7e 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -147,6 +147,12 @@ enum drm_mode_status {
#define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
+#define DRM_MODE_MATCH_TIMINGS (1 << 0) +#define DRM_MODE_MATCH_CLOCK (1 << 1) +#define DRM_MODE_MATCH_FLAGS (1 << 2) +#define DRM_MODE_MATCH_3D_FLAGS (1 << 3) +#define DRM_MODE_MATCH_ASPECT_RATIO (1 << 4) + /** * struct drm_display_mode - DRM kernel-internal display mode structure * @hdisplay: horizontal display size @@ -490,6 +496,9 @@ void drm_mode_copy(struct drm_display_mode *dst, const struct drm_display_mode *src); struct drm_display_mode *drm_mode_duplicate(struct drm_device *dev, const struct drm_display_mode *mode); +bool drm_mode_match(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2, + unsigned int match_flags); bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1,
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use drm_mode_equal_no_clocks_no_stereo() in drm_match_hdmi_mode_clock_tolerance() for consistency as we also use it in drm_match_hdmi_mode() and the cea mode matching functions.
This doesn't actually change anything since the input mode comes from detailed timings and we match it against edid_4k_modes[] which. So none of those modes can have stereo flags set.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/drm_edid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 788fee4..cfd8ead 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3048,7 +3048,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ abs(to_match->clock - clock2) > clock_tolerance) continue;
- if (drm_mode_equal_no_clocks(to_match, hdmi_mode)) + if (drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) return vic; }
From: Ville Syrjälä ville.syrjala@linux.intel.com
commit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer") cause us to not send out any VICs in the AVI infoframes. That commit was since reverted, but if and when we add aspect ratio handing back we need to be more careful.
Let's handle this by considering the aspect ratio as a requirement for cea mode matching only if the passed in mode actually has a non-zero aspect ratio field. This will keep userspace that doesn't provide an aspect ratio working as before by matching it to the first otherwise equal cea mode. And once userspace starts to provide the aspect ratio it will be considerd a hard requirement for the match.
Also change the hdmi mode matching to use drm_mode_match() for consistency, but we don't match on aspect ratio there since the spec doesn't list a specific aspect ratio for those modes.
Cc: Shashank Sharma shashank.sharma@intel.com Cc: "Lin, Jia" lin.a.jia@intel.com Cc: Akashdeep Sharma akashdeep.sharma@intel.com Cc: Jim Bride jim.bride@linux.intel.com Cc: Jose Abreu Jose.Abreu@synopsys.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Emil Velikov emil.l.velikov@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index cfd8ead..b635fca 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2931,11 +2931,15 @@ cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode) static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS; u8 vic;
if (!to_match->clock) return 0;
+ if (to_match->picture_aspect_ratio) + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { struct drm_display_mode cea_mode = edid_cea_modes[vic]; unsigned int clock1, clock2; @@ -2949,7 +2953,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m continue;
do { - if (drm_mode_equal_no_clocks_no_stereo(to_match, &cea_mode)) + if (drm_mode_match(to_match, &cea_mode, match_flags)) return vic; } while (cea_mode_alternate_timings(vic, &cea_mode)); } @@ -2966,11 +2970,15 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) { + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS; u8 vic;
if (!to_match->clock) return 0;
+ if (to_match->picture_aspect_ratio) + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { struct drm_display_mode cea_mode = edid_cea_modes[vic]; unsigned int clock1, clock2; @@ -2984,7 +2992,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) continue;
do { - if (drm_mode_equal_no_clocks_no_stereo(to_match, &cea_mode)) + if (drm_mode_match(to_match, &cea_mode, match_flags)) return vic; } while (cea_mode_alternate_timings(vic, &cea_mode)); } @@ -3031,6 +3039,7 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode) static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS; u8 vic;
if (!to_match->clock) @@ -3048,7 +3057,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ abs(to_match->clock - clock2) > clock_tolerance) continue;
- if (drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) + if (drm_mode_match(to_match, hdmi_mode, match_flags)) return vic; }
@@ -3065,6 +3074,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ */ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) { + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS; u8 vic;
if (!to_match->clock) @@ -3080,7 +3090,7 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && - drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) + drm_mode_match(to_match, hdmi_mode, match_flags)) return vic; } return 0;
From: Ville Syrjälä ville.syrjala@linux.intel.com
If the user mode would specify an aspect ratio other than 4:3 or 16:9 we now silently ignore it. Maybe a better apporoach is to return an error? Let's try that.
Also we must be careful that we don't try to send illegal picture aspect in the infoframe as it's only capable of signalling none, 4:3, and 16:9. Currently we're sending these bogus infoframes whenever the cea mode specifies some other aspect ratio.
Cc: Shashank Sharma shashank.sharma@intel.com Cc: Sean Paul seanpaul@chromium.org Cc: Jose Abreu Jose.Abreu@synopsys.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Emil Velikov emil.l.velikov@gmail.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b635fca..8d87ab3 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4841,6 +4841,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, const struct drm_display_mode *mode, bool is_hdmi2_sink) { + enum hdmi_picture_aspect picture_aspect; int err;
if (!frame || !mode) @@ -4883,13 +4884,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, * Populate picture aspect ratio from either * user input (if specified) or from the CEA mode list. */ - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) - frame->picture_aspect = mode->picture_aspect_ratio; - else if (frame->video_code > 0) - frame->picture_aspect = drm_get_cea_aspect_ratio( - frame->video_code); + picture_aspect = mode->picture_aspect_ratio; + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code);
+ /* + * The infoframe can't convey anything but none, 4:3 + * and 16:9, so if the user has asked for anything else + * we can only satisfy it by specifying the right VIC. + */ + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) { + if (picture_aspect != + drm_get_cea_aspect_ratio(frame->video_code)) + return -EINVAL; + picture_aspect = HDMI_PICTURE_ASPECT_NONE; + } + + frame->picture_aspect = picture_aspect; frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
From: Ville Syrjälä ville.syrjala@linux.intel.com
AVI infoframe can only carry none, 4:3, or 16:9 picture aspect ratios. Return an error if the user asked for something different.
Cc: Shashank Sharma shashank.sharma@intel.com Cc: "Lin, Jia" lin.a.jia@intel.com Cc: Akashdeep Sharma akashdeep.sharma@intel.com Cc: Jim Bride jim.bride@linux.intel.com Cc: Jose Abreu Jose.Abreu@synopsys.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Emil Velikov emil.l.velikov@gmail.com Cc: Thierry Reding thierry.reding@gmail.com Cc: Hans Verkuil hans.verkuil@cisco.com Cc: linux-media@vger.kernel.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Jose Abreu joabreu@synopsys.com --- drivers/video/hdmi.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index 111a0ab..38716eb5 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -93,6 +93,9 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer, if (size < length) return -ENOSPC;
+ if (frame->picture_aspect > HDMI_PICTURE_ASPECT_16_9) + return -EINVAL; + memset(buffer, 0, size);
ptr[0] = frame->type;
From: Ankit Nautiyal ankit.k.nautiyal@intel.com
To enable aspect-ratio support in DRM, blindly exposing the aspect ratio information along with mode, can break things in existing user-spaces which have no intention or support to use this aspect ratio information.
To avoid this, a new drm client cap is required to enable a user-space to advertise if it supports modes with aspect-ratio. Based on this cap value, the kernel will take a call on exposing the aspect ratio info in modes or not.
This patch adds the client cap for aspect-ratio.
Cc: Ville Syrjala ville.syrjala@linux.intel.com Cc: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Ankit Nautiyal ankit.k.nautiyal@intel.com
V3: rebase V4: As suggested by Marteen Lankhorst modified the commit message explaining the need to use the DRM cap for aspect-ratio. Also, tweaked the comment lines in the code for better understanding and clarity, as recommended by Shashank Sharma. V5: rebase V6: rebase V7: rebase V8: rebase
Reviewed-by: Shashank Sharma shashank.sharma@intel.com --- drivers/gpu/drm/drm_ioctl.c | 5 +++++ include/drm/drm_file.h | 8 ++++++++ include/uapi/drm/drm.h | 7 +++++++ 3 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index af78291..54a98b7 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -325,6 +325,11 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) file_priv->atomic = req->value; file_priv->universal_planes = req->value; break; + case DRM_CLIENT_CAP_ASPECT_RATIO: + if (req->value > 1) + return -EINVAL; + file_priv->aspect_ratio_allowed = req->value; + break; default: return -EINVAL; } diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 5176c37..02b7dde 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -182,6 +182,14 @@ struct drm_file { unsigned atomic:1;
/** + * @aspect_ratio_allowed: + * + * True, if client can handle picture aspect ratios, and has requested + * to pass this information along with the mode. + */ + unsigned aspect_ratio_allowed:1; + + /** * @is_master: * * This client is the creator of @master. Protected by struct diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 6fdff59..9c660e1 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -680,6 +680,13 @@ struct drm_get_cap { */ #define DRM_CLIENT_CAP_ATOMIC 3
+/** + * DRM_CLIENT_CAP_ASPECT_RATIO + * + * If set to 1, the DRM core will provide aspect ratio information in modes. + */ +#define DRM_CLIENT_CAP_ASPECT_RATIO 4 + /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */ struct drm_set_client_cap { __u64 capability;
From: Ankit Nautiyal ankit.k.nautiyal@intel.com
If the user-space does not support aspect-ratio, then getblob called with the blob id of a user-mode, should clear the aspect-ratio information in the blob data.
Currently for a given blob id, there is no way to determine if the blob stores user-mode or not. This can only be ascertained when the blob is used for an atomic modeset call.
This patch: -adds a new field 'is_video_mode' in drm_property_blob to differentiate between the video mode blobs and the other blobs. -sets the field 'is_video_mode' when the blob is used for modeset. -removes the aspect-ratio info from the user-mode data if aspect-ratio is not supported by the user, while returning the blob to the user, in getblob ioctl.
Signed-off-by: Ankit Nautiyal ankit.k.nautiyal@intel.com
V5: This patch is introduced in the rev-5 of the series. V6: As suggested by Ville: -added helper functions for determining if aspect-ratio is expected in user-mode and for allowing/disallowing the aspect-ratio, if its not expected. -avoided clobbering of blob-data, instead cleared the aspect-ratio in the user-mode only, so that another client with aspect-ratio cap, can still get the aspect-ratio information from getblob. V7: Fixed warning [Wint-to-pointer-cast] for 32 bit platforms. V8: Changed the parameter of aspect-ratio helper functions from 32 bit flags to user-mode, to avoid passing random integers, as suggested by Ville. --- drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_modes.c | 47 ++++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_property.c | 5 +++++ include/drm/drm_modes.h | 4 ++++ include/drm/drm_property.h | 2 ++ 5 files changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 34b7d42..2b1c88a 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -464,6 +464,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, else if (property == config->prop_mode_id) { struct drm_property_blob *mode = drm_property_lookup_blob(dev, val); + mode->is_video_mode = true; ret = drm_atomic_set_mode_prop_for_crtc(state, mode); drm_property_blob_put(mode); return ret; diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index a48672c..36e2abc 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1761,3 +1761,50 @@ bool drm_mode_is_420(const struct drm_display_info *display, drm_mode_is_420_also(display, mode); } EXPORT_SYMBOL(drm_mode_is_420); + +/** + * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information + * is expected from the user-mode. + * + * If the user has set aspect-ratio cap, then the flag of the user-mode is + * allowed to contain aspect-ratio value. + * If the user does not set aspect-ratio cap, then the only value allowed in the + * flags bits is aspect-ratio NONE. + * + * @file_priv: file private structure to get the user capabilities + * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio + * information. + * + * Returns: + * true if the aspect-ratio info is allowed in the user-mode flags. + * false, otherwise. + */ +bool +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv, + struct drm_mode_modeinfo *umode) +{ + return file_priv->aspect_ratio_allowed || (umode->flags & + DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE; +} +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed); + +/** + * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the + * user-mode flags. + * + * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio + * bits in the user-mode flags, if aspect-ratio info is not allowed. + * + * @file_priv: file private structure to get the user capabilities. + * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to + * be filtered. + * + */ +void +drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv, + struct drm_mode_modeinfo *umode) +{ + if (!drm_mode_aspect_ratio_allowed(file_priv, umode)) + umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK; +} +EXPORT_SYMBOL(drm_mode_filter_aspect_ratio_flags); diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index 6ac6ee4..664cb57 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -770,6 +770,11 @@ int drm_mode_getblob_ioctl(struct drm_device *dev, ret = -EFAULT; goto unref; } + if (blob->is_video_mode) { + struct drm_mode_modeinfo __user *mode = + u64_to_user_ptr(out_resp->data); + drm_mode_filter_aspect_ratio_flags(file_priv, mode); + } } out_resp->length = blob->length; unref: diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 2f78b7e..e0b060d 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display, const struct drm_display_mode *mode); bool drm_mode_is_420(const struct drm_display_info *display, const struct drm_display_mode *mode); +bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv, + struct drm_mode_modeinfo *umode); +void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv, + struct drm_mode_modeinfo *umode);
struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh, diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h index 937757a..ce374e7 100644 --- a/include/drm/drm_property.h +++ b/include/drm/drm_property.h @@ -194,6 +194,7 @@ struct drm_property { * @head_global: entry on the global blob list in * &drm_mode_config.property_blob_list. * @head_file: entry on the per-file blob list in &drm_file.blobs list. + * @is_video_mode: flag to mark the blobs that contain drm_mode_modeinfo. * @length: size of the blob in bytes, invariant over the lifetime of the object * @data: actual data, embedded at the end of this structure * @@ -208,6 +209,7 @@ struct drm_property_blob { struct drm_device *dev; struct list_head head_global; struct list_head head_file; + bool is_video_mode; size_t length; unsigned char data[]; };
Hi Ankit,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on next-20180309] [also build test WARNING on v4.16-rc5] [cannot apply to linus/master v4.16-rc4 v4.16-rc3 v4.16-rc2] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nautiyal-Ankit-K/Aspect-ratio-suppo... reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
drivers/gpu/drm/drm_property.c:776:71: sparse: incorrect type in argument 2 (different address spaces) @@ expected struct drm_mode_modeinfo *umode @@ got struct drm_mode_mstruct drm_mode_modeinfo *umode @@
drivers/gpu/drm/drm_property.c:776:71: expected struct drm_mode_modeinfo *umode drivers/gpu/drm/drm_property.c:776:71: got struct drm_mode_modeinfo [noderef] asn:1*mode
vim +776 drivers/gpu/drm/drm_property.c
751 752 int drm_mode_getblob_ioctl(struct drm_device *dev, 753 void *data, struct drm_file *file_priv) 754 { 755 struct drm_mode_get_blob *out_resp = data; 756 struct drm_property_blob *blob; 757 int ret = 0; 758 759 if (!drm_core_check_feature(dev, DRIVER_MODESET)) 760 return -EINVAL; 761 762 blob = drm_property_lookup_blob(dev, out_resp->blob_id); 763 if (!blob) 764 return -ENOENT; 765 766 if (out_resp->length == blob->length) { 767 if (copy_to_user(u64_to_user_ptr(out_resp->data), 768 blob->data, 769 blob->length)) { 770 ret = -EFAULT; 771 goto unref; 772 } 773 if (blob->is_video_mode) { 774 struct drm_mode_modeinfo __user *mode = 775 u64_to_user_ptr(out_resp->data);
776 drm_mode_filter_aspect_ratio_flags(file_priv, mode);
777 } 778 } 779 out_resp->length = blob->length; 780 unref: 781 drm_property_blob_put(blob); 782 783 return ret; 784 } 785
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
From: Ankit Nautiyal ankit.k.nautiyal@intel.com
If the user-space does not support aspect-ratio, and requests for a modeset with mode having aspect ratio bits set, then the given user-mode must be rejected. Secondly, while preparing a user-mode from kernel mode, the aspect-ratio info must not be given, if aspect-ratio is not supported by the user.
This patch: 1. passes the file_priv structure from the drm_mode_atomic_ioctl till the drm_mode_crtc_set_mode_prop, to get the user capability. 2. rejects the modes with aspect-ratio info, during modeset, if the user does not support aspect ratio. 3. does not load the aspect-ratio info in user-mode structure, if aspect ratio is not supported.
Signed-off-by: Ankit Nautiyal ankit.k.nautiyal@intel.com
V3: Addressed review comments from Ville: Do not corrupt the current crtc state by updating aspect-ratio on the fly. V4: rebase V5: As suggested by Ville, rejected the modeset calls for modes with aspect ratio, if the user does not set aspect-ratio cap. V6: Used the helper functions for determining if aspect-ratio is expected in the user-mode. V7: rebase V8: rebase --- drivers/gpu/drm/drm_atomic.c | 35 +++++++++++++++++++++++++---------- drivers/gpu/drm/drm_atomic_helper.c | 6 +++--- drivers/gpu/drm/drm_crtc.c | 8 ++++++++ drivers/gpu/drm/drm_crtc_internal.h | 3 ++- drivers/gpu/drm/drm_mode_object.c | 9 ++++++--- include/drm/drm_atomic.h | 5 +++-- 6 files changed, 47 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 2b1c88a..2155aa4 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -368,6 +368,7 @@ EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc); * drm_atomic_set_mode_prop_for_crtc - set mode for CRTC * @state: the CRTC whose incoming state to update * @blob: pointer to blob property to use for mode + * @file_priv: file priv structure, to get the userspace capabilities * * Set a mode (originating from a blob property) on the desired CRTC state. * This function will take a reference on the blob property for the CRTC state, @@ -378,7 +379,8 @@ EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc); * Zero on success, error code on failure. Cannot return -EDEADLK. */ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, - struct drm_property_blob *blob) + struct drm_property_blob *blob, + struct drm_file *file_priv) { if (blob == state->mode_blob) return 0; @@ -389,10 +391,21 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, memset(&state->mode, 0, sizeof(state->mode));
if (blob) { - if (blob->length != sizeof(struct drm_mode_modeinfo) || - drm_mode_convert_umode(state->crtc->dev, &state->mode, - (const struct drm_mode_modeinfo *) - blob->data)) + struct drm_mode_modeinfo *u_mode; + + if (blob->length != sizeof(struct drm_mode_modeinfo)) + return -EINVAL; + + u_mode = (struct drm_mode_modeinfo *) blob->data; + if (!drm_mode_aspect_ratio_allowed(file_priv, + u_mode)) { + DRM_DEBUG_ATOMIC("Unexpected aspect-ratio flag bits\n"); + return -EINVAL; + } + + if (drm_mode_convert_umode(state->crtc->dev, &state->mode, + (const struct drm_mode_modeinfo *) + u_mode)) return -EINVAL;
state->mode_blob = drm_property_blob_get(blob); @@ -441,6 +454,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev, * @state: the state object to update with the new property value * @property: the property to set * @val: the new property value + * @file_priv: the file private structure, to get the user capabilities * * This function handles generic/core properties and calls out to driver's * &drm_crtc_funcs.atomic_set_property for driver properties. To ensure @@ -452,7 +466,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev, */ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, struct drm_crtc_state *state, struct drm_property *property, - uint64_t val) + uint64_t val, struct drm_file *file_priv) { struct drm_device *dev = crtc->dev; struct drm_mode_config *config = &dev->mode_config; @@ -465,7 +479,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, struct drm_property_blob *mode = drm_property_lookup_blob(dev, val); mode->is_video_mode = true; - ret = drm_atomic_set_mode_prop_for_crtc(state, mode); + ret = drm_atomic_set_mode_prop_for_crtc(state, mode, file_priv); drm_property_blob_put(mode); return ret; } else if (property == config->degamma_lut_property) { @@ -1932,7 +1946,8 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state, int drm_atomic_set_property(struct drm_atomic_state *state, struct drm_mode_object *obj, struct drm_property *prop, - uint64_t prop_value) + uint64_t prop_value, + struct drm_file *file_priv) { struct drm_mode_object *ref; int ret; @@ -1966,7 +1981,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state, }
ret = drm_atomic_crtc_set_property(crtc, - crtc_state, prop, prop_value); + crtc_state, prop, prop_value, file_priv); break; } case DRM_MODE_OBJECT_PLANE: { @@ -2355,7 +2370,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, }
ret = drm_atomic_set_property(state, obj, prop, - prop_value); + prop_value, file_priv); if (ret) { drm_mode_object_put(obj); goto out; diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 00c78c1..21d6455 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -186,7 +186,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
if (!crtc_state->connector_mask) { ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, - NULL); + NULL, NULL); if (ret < 0) goto out;
@@ -2751,7 +2751,7 @@ static int update_output_state(struct drm_atomic_state *state,
if (!new_crtc_state->connector_mask) { ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state, - NULL); + NULL, NULL); if (ret < 0) return ret;
@@ -2932,7 +2932,7 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
crtc_state->active = false;
- ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL); + ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL, NULL); if (ret < 0) goto free;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0358388..c35561e 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -446,6 +446,7 @@ int drm_mode_getcrtc(struct drm_device *dev, crtc_resp->mode_valid = 0; } } + drm_mode_filter_aspect_ratio_flags(file_priv, &crtc_resp->mode); drm_modeset_unlock(&crtc->mutex);
return 0; @@ -613,6 +614,13 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, ret = -ENOMEM; goto out; } + if (!drm_mode_aspect_ratio_allowed(file_priv, + &crtc_req->mode)) { + DRM_DEBUG_KMS("Unexpected aspect-ratio flag bits\n"); + ret = -EINVAL; + goto out; + } +
ret = drm_mode_convert_umode(dev, mode, &crtc_req->mode); if (ret) { diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index 3c2b828..7e9f8f8 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -188,7 +188,8 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state, int drm_atomic_set_property(struct drm_atomic_state *state, struct drm_mode_object *obj, struct drm_property *prop, - uint64_t prop_value); + uint64_t prop_value, + struct drm_file *file_priv); int drm_atomic_get_property(struct drm_mode_object *obj, struct drm_property *property, uint64_t *val); int drm_mode_atomic_ioctl(struct drm_device *dev, diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index ce4d2fb..1c3d498 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -452,7 +452,8 @@ static int set_property_legacy(struct drm_mode_object *obj,
static int set_property_atomic(struct drm_mode_object *obj, struct drm_property *prop, - uint64_t prop_value) + uint64_t prop_value, + struct drm_file *file_priv) { struct drm_device *dev = prop->dev; struct drm_atomic_state *state; @@ -476,7 +477,8 @@ static int set_property_atomic(struct drm_mode_object *obj, obj_to_connector(obj), prop_value); } else { - ret = drm_atomic_set_property(state, obj, prop, prop_value); + ret = drm_atomic_set_property(state, obj, prop, prop_value, + file_priv); if (ret) goto out; ret = drm_atomic_commit(state); @@ -519,7 +521,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data, goto out_unref;
if (drm_drv_uses_atomic_modeset(property->dev)) - ret = set_property_atomic(arg_obj, property, arg->value); + ret = set_property_atomic(arg_obj, property, arg->value, + file_priv); else ret = set_property_legacy(arg_obj, property, arg->value);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index a57a8aa..b7106f7 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -367,7 +367,7 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state, struct drm_crtc *crtc); int drm_atomic_crtc_set_property(struct drm_crtc *crtc, struct drm_crtc_state *state, struct drm_property *property, - uint64_t val); + uint64_t val, struct drm_file *file_priv); struct drm_plane_state * __must_check drm_atomic_get_plane_state(struct drm_atomic_state *state, struct drm_plane *plane); @@ -583,7 +583,8 @@ drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, const struct drm_display_mode *mode); int __must_check drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, - struct drm_property_blob *blob); + struct drm_property_blob *blob, + struct drm_file *file_priv); int __must_check drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, struct drm_crtc *crtc);
From: Ankit Nautiyal ankit.k.nautiyal@intel.com
We parse the EDID and add all the modes in the connector's modelist. This adds CEA modes with aspect ratio information too, regadless of whether user space requested this information or not.
This patch prunes the modes with aspect-ratio information, from a connector's modelist, if the user-space has not set the aspect ratio DRM client cap.
Cc: Ville Syrjala ville.syrjala@linux.intel.com Cc: Shashank Sharma shashank.sharma@intel.com Cc: Jose Abreu jose.abreu@synopsys.com
Signed-off-by: Ankit Nautiyal ankit.k.nautiyal@intel.com
V3: As suggested by Ville, modified the mechanism of pruning of modes with aspect-ratio, if the aspect-ratio is not supported. Instead of straight away pruning such a mode, the mode is retained with aspect ratio bits set to zero, provided it is unique. V4: rebase V5: Addressed review comments from Ville: -used a pointer to store last valid mode. -avoided, modifying of picture_aspect_ratio in kernel mode, instead only flags bits of user mode are reset (if aspect-ratio is not supported). V6: As suggested by Ville, corrected the mode pruning logic and elaborated the mode pruning logic and the assumptions taken. V7: rebase V8: rebase --- drivers/gpu/drm/drm_connector.c | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index b3cde89..5420325 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1531,8 +1531,10 @@ static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne return connector->encoder; }
-static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, - const struct drm_file *file_priv) +static bool +drm_mode_expose_to_userspace(const struct drm_display_mode *mode, + const struct drm_display_mode *last_mode, + const struct drm_file *file_priv) { /* * If user-space hasn't configured the driver to expose the stereo 3D @@ -1540,6 +1542,26 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, */ if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode)) return false; + /* + * If user-space hasn't configured the driver to expose the modes with + * aspect-ratio, don't expose them. But in case of a unique mode, let + * the mode be passed, so that it can be enumerated with aspect-ratio + * bits erased. + * + * It is assumed here, that the list of modes for a given connector, is + * sorted, such that modes that have different aspect-ratios, but are + * otherwise identical, are back to back. + * This way, saving the last valid mode, and matching it with the + * current mode will help in determining, if the current mode is unique. + */ + if (!file_priv->aspect_ratio_allowed && + mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE && + last_mode && drm_mode_match(mode, last_mode, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_CLOCK | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS)) + return false;
return true; } @@ -1551,6 +1573,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, struct drm_connector *connector; struct drm_encoder *encoder; struct drm_display_mode *mode; + struct drm_display_mode *last_valid_mode; int mode_count = 0; int encoders_count = 0; int ret = 0; @@ -1606,9 +1629,13 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, out_resp->connection = connector->status;
/* delayed so we get modes regardless of pre-fill_modes state */ + last_valid_mode = NULL; list_for_each_entry(mode, &connector->modes, head) - if (drm_mode_expose_to_userspace(mode, file_priv)) + if (drm_mode_expose_to_userspace(mode, last_valid_mode, + file_priv)) { mode_count++; + last_valid_mode = mode; + }
/* * This ioctl is called twice, once to determine how much space is @@ -1617,11 +1644,15 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, if ((out_resp->count_modes >= mode_count) && mode_count) { copied = 0; mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr; + last_valid_mode = NULL; list_for_each_entry(mode, &connector->modes, head) { - if (!drm_mode_expose_to_userspace(mode, file_priv)) + if (!drm_mode_expose_to_userspace(mode, + last_valid_mode, + file_priv)) continue;
drm_mode_convert_to_umode(&u_mode, mode); + drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode); if (copy_to_user(mode_ptr + copied, &u_mode, sizeof(u_mode))) { ret = -EFAULT; @@ -1629,6 +1660,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
goto out; } + last_valid_mode = mode; copied++; } }
From: "Sharma, Shashank" shashank.sharma@intel.com
Current DRM layer functions don't parse aspect ratio information while converting a user mode->kernel mode or vice versa. This causes modeset to pick mode with wrong aspect ratio, eventually causing failures in HDMI compliance test cases, due to wrong VIC.
This patch adds aspect ratio information in DRM's mode conversion and mode comparision functions, to make sure kernel picks mode with right aspect ratio (as per the VIC).
Background: This patch was once reviewed and merged, and later reverted due to lack of DRM cap protection. This is a re-spin of this patch, this time with DRM cap protection, to avoid aspect ratio information, when the client doesn't request for it.
Review link: https://pw-emeril.freedesktop.org/patch/104068/ Background discussion: https://patchwork.kernel.org/patch/9379057/
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Lin, Jia lin.a.jia@intel.com Signed-off-by: Akashdeep Sharma akashdeep.sharma@intel.com Reviewed-by: Jim Bride jim.bride@linux.intel.com (V2) Reviewed-by: Jose Abreu Jose.Abreu@synopsys.com (V4)
Cc: Ville Syrjala ville.syrjala@linux.intel.com Cc: Jim Bride jim.bride@linux.intel.com Cc: Jose Abreu Jose.Abreu@synopsys.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com
V3: modified the aspect-ratio check in drm_mode_equal as per new flags provided by Ville. https://patchwork.freedesktop.org/patch/188043/ V4: rebase V5: rebase V6: As recommended by Ville, avoided matching of aspect-ratio in drm_fb_helper, while trying to find a common mode among connectors for the target clone mode. V7: rebase V8: rebase --- drivers/gpu/drm/drm_fb_helper.c | 12 ++++++++++-- drivers/gpu/drm/drm_modes.c | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 035784d..8fa4e78 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper *fb_helper, for (j = 0; j < i; j++) { if (!enabled[j]) continue; - if (!drm_mode_equal(modes[j], modes[i])) + if (!drm_mode_match(modes[j], modes[i], + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_CLOCK | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS)) can_clone = false; } } @@ -2203,7 +2207,11 @@ static bool drm_target_cloned(struct drm_fb_helper *fb_helper,
fb_helper_conn = fb_helper->connector_info[i]; list_for_each_entry(mode, &fb_helper_conn->connector->modes, head) { - if (drm_mode_equal(mode, dmt_mode)) + if (drm_mode_match(mode, dmt_mode, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_CLOCK | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS)) modes[i] = mode; } if (!modes[i]) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 36e2abc..9dc54ce 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1050,7 +1050,8 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_CLOCK | DRM_MODE_MATCH_FLAGS | - DRM_MODE_MATCH_3D_FLAGS); + DRM_MODE_MATCH_3D_FLAGS| + DRM_MODE_MATCH_ASPECT_RATIO); } EXPORT_SYMBOL(drm_mode_equal);
@@ -1648,6 +1649,20 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, out->vrefresh = in->vrefresh; out->flags = in->flags; out->type = in->type; + + switch (in->picture_aspect_ratio) { + case HDMI_PICTURE_ASPECT_4_3: + out->flags |= DRM_MODE_FLAG_PIC_AR_4_3; + break; + case HDMI_PICTURE_ASPECT_16_9: + out->flags |= DRM_MODE_FLAG_PIC_AR_16_9; + break; + case HDMI_PICTURE_ASPECT_RESERVED: + default: + out->flags |= DRM_MODE_FLAG_PIC_AR_NONE; + break; + } + strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); out->name[DRM_DISPLAY_MODE_LEN-1] = 0; } @@ -1692,6 +1707,24 @@ int drm_mode_convert_umode(struct drm_device *dev, strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
+ /* Clearing picture aspect ratio bits from out flags, + * as the aspect-ratio information is not stored in + * flags for kernel-mode, but in picture_aspect_ratio. + */ + out->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK; + + switch (in->flags & DRM_MODE_FLAG_PIC_AR_MASK) { + case DRM_MODE_FLAG_PIC_AR_4_3: + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3; + break; + case DRM_MODE_FLAG_PIC_AR_16_9: + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9; + break; + default: + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; + break; + } + out->status = drm_mode_validate_driver(dev, out); if (out->status != MODE_OK) goto out;
From: "Sharma, Shashank" shashank.sharma@intel.com
HDMI 2.0/CEA-861-F introduces two new aspect ratios: - 64:27 - 256:135
This patch: - Adds new DRM flags for to represent these new aspect ratios. - Adds new cases to handle these aspect ratios while converting from user->kernel mode or vise versa.
This patch was once reviewed and merged, and later reverted due to lack of DRM client protection, while adding aspect ratio bits in user modes. This is a re-spin of the series, with DRM client cap protection.
The previous series can be found here: https://pw-emeril.freedesktop.org/series/10850/
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Reviewed-by: Sean Paul seanpaul@chromium.org (V2) Reviewed-by: Jose Abreu Jose.Abreu@synopsys.com (V2)
Cc: Ville Syrjala ville.syrjala@linux.intel.com Cc: Sean Paul seanpaul@chromium.org Cc: Jose Abreu Jose.Abreu@synopsys.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com
V3: rebase V4: rebase V5: corrected the macro name for an aspect ratio, in a switch case. V6: rebase V7: rebase V8: rebase --- drivers/gpu/drm/drm_modes.c | 12 ++++++++++++ include/uapi/drm/drm_mode.h | 6 ++++++ 2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 9dc54ce..76abe4d 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1657,6 +1657,12 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, case HDMI_PICTURE_ASPECT_16_9: out->flags |= DRM_MODE_FLAG_PIC_AR_16_9; break; + case HDMI_PICTURE_ASPECT_64_27: + out->flags |= DRM_MODE_FLAG_PIC_AR_64_27; + break; + case HDMI_PICTURE_ASPECT_256_135: + out->flags |= DRM_MODE_FLAG_PIC_AR_256_135; + break; case HDMI_PICTURE_ASPECT_RESERVED: default: out->flags |= DRM_MODE_FLAG_PIC_AR_NONE; @@ -1720,6 +1726,12 @@ int drm_mode_convert_umode(struct drm_device *dev, case DRM_MODE_FLAG_PIC_AR_16_9: out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9; break; + case DRM_MODE_FLAG_PIC_AR_64_27: + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_64_27; + break; + case DRM_MODE_FLAG_PIC_AR_256_135: + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_256_135; + break; default: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; break; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 50bcf42..4b3a1bb 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -93,6 +93,8 @@ extern "C" { #define DRM_MODE_PICTURE_ASPECT_NONE 0 #define DRM_MODE_PICTURE_ASPECT_4_3 1 #define DRM_MODE_PICTURE_ASPECT_16_9 2 +#define DRM_MODE_PICTURE_ASPECT_64_27 3 +#define DRM_MODE_PICTURE_ASPECT_256_135 4
/* Aspect ratio flag bitmask (4 bits 22:19) */ #define DRM_MODE_FLAG_PIC_AR_MASK (0x0F<<19) @@ -102,6 +104,10 @@ extern "C" { (DRM_MODE_PICTURE_ASPECT_4_3<<19) #define DRM_MODE_FLAG_PIC_AR_16_9 \ (DRM_MODE_PICTURE_ASPECT_16_9<<19) +#define DRM_MODE_FLAG_PIC_AR_64_27 \ + (DRM_MODE_PICTURE_ASPECT_64_27<<19) +#define DRM_MODE_FLAG_PIC_AR_256_135 \ + (DRM_MODE_PICTURE_ASPECT_256_135<<19)
#define DRM_MODE_FLAG_ALL (DRM_MODE_FLAG_PHSYNC | \ DRM_MODE_FLAG_NHSYNC | \
dri-devel@lists.freedesktop.org
-
kbuild test robot -
Nautiyal, Ankit K