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 3 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.
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ä (3): 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
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 | 18 ++- 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 | 223 +++++++++++++++++++++++++++++++----- drivers/gpu/drm/drm_property.c | 5 + 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 + 17 files changed, 346 insertions(+), 60 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: 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
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
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. --- drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_modes.c | 44 ++++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_property.c | 5 +++++ include/drm/drm_modes.h | 4 ++++ include/drm/drm_property.h | 2 ++ 5 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 46733d5..86b483e 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..219890f 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1761,3 +1761,47 @@ 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 + * @flag: 32 bit flag that contains 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, uint32_t flags) +{ + return file_priv->aspect_ratio_allowed || + (flags & DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE; +} +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed); + +/** + * drm_mode_handle_aspect_ratio - handles 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 flag, if aspect-ratio info is not allowed. + * + * @file_priv: file private structure to get the user capabilities. + * @flag: 32 bit flag that is to be modified, in case the aspect + * ratio info is not allowed. + * + */ +void +drm_mode_handle_aspect_ratio(const struct drm_file *file_priv, uint32_t *flags) +{ + if (!drm_mode_aspect_ratio_allowed(file_priv, *flags)) + *flags &= ~DRM_MODE_FLAG_PIC_AR_MASK; +} +EXPORT_SYMBOL(drm_mode_handle_aspect_ratio); diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index bae50e6..4e56f84 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -753,6 +753,11 @@ int drm_mode_getblob_ioctl(struct drm_device *dev, ret = -EFAULT; goto unref; } + if (blob->is_video_mode) { + struct drm_mode_modeinfo *mode = + (struct drm_mode_modeinfo *) out_resp->data; + drm_mode_handle_aspect_ratio(file_priv, &mode->flags); + } } out_resp->length = blob->length; unref: diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 2f78b7e..51d1188 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, + uint32_t flags); +void drm_mode_handle_aspect_ratio(const struct drm_file *file_priv, + uint32_t *flags);
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 8a522b4..95e6e32 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 drm/drm-next] [also build test WARNING on v4.16-rc4 next-20180306] [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... base: git://people.freedesktop.org/~airlied/linux.git drm-next config: i386-randconfig-x015-201809 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386
All warnings (new ones prefixed by >>):
drivers/gpu/drm/drm_property.c: In function 'drm_mode_getblob_ioctl':
drivers/gpu/drm/drm_property.c:758:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
(struct drm_mode_modeinfo *) out_resp->data; ^
vim +758 drivers/gpu/drm/drm_property.c
734 735 int drm_mode_getblob_ioctl(struct drm_device *dev, 736 void *data, struct drm_file *file_priv) 737 { 738 struct drm_mode_get_blob *out_resp = data; 739 struct drm_property_blob *blob; 740 int ret = 0; 741 742 if (!drm_core_check_feature(dev, DRIVER_MODESET)) 743 return -EINVAL; 744 745 blob = drm_property_lookup_blob(dev, out_resp->blob_id); 746 if (!blob) 747 return -ENOENT; 748 749 if (out_resp->length == blob->length) { 750 if (copy_to_user(u64_to_user_ptr(out_resp->data), 751 blob->data, 752 blob->length)) { 753 ret = -EFAULT; 754 goto unref; 755 } 756 if (blob->is_video_mode) { 757 struct drm_mode_modeinfo *mode =
758 (struct drm_mode_modeinfo *) out_resp->data;
759 drm_mode_handle_aspect_ratio(file_priv, &mode->flags); 760 } 761 } 762 out_resp->length = blob->length; 763 unref: 764 drm_property_blob_put(blob); 765 766 return ret; 767 } 768
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Ankit,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next] [also build test WARNING on v4.16-rc4 next-20180306] [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... base: git://people.freedesktop.org/~airlied/linux.git drm-next reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.blkcipher' not described in 'crypto_alg' include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.cipher' not described in 'crypto_alg' include/linux/crypto.h:477: warning: Function parameter or member 'cra_u.compress' not described in 'crypto_alg' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.prev_bssid' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev' include/net/cfg80211.h:4129: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev' include/net/mac80211.h:2259: warning: Function parameter or member 'radiotap_timestamp.units_pos' not described in 'ieee80211_hw' include/net/mac80211.h:2259: warning: Function parameter or member 'radiotap_timestamp.accuracy' not described in 'ieee80211_hw' include/net/mac80211.h:950: warning: Function parameter or member 'rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.use_rts' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.use_cts_prot' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.short_preamble' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.skip_table' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.jiffies' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.vif' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.hw_key' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.flags' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'control.enqueue_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'ack' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'status_driver_data' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info' include/net/mac80211.h:950: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'msdu_retries' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'msdu_failed' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info' net/mac80211/sta_info.h:584: warning: Function parameter or member 'msdu' not described in 'sta_info' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf' include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array' include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.sign' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.realbits' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.storagebits' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.shift' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.repeat' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.endianness' not described in 'iio_chan_spec' include/linux/iio/hw-consumer.h:1: warning: no structured comments found include/linux/device.h:294: warning: Function parameter or member 'coredump' not described in 'device_driver' include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry' include/linux/mtd/rawnand.h:709: warning: Function parameter or member 'timings.sdr' not described in 'nand_data_interface' include/linux/mtd/rawnand.h:774: warning: Function parameter or member 'buf' not described in 'nand_op_data_instr' include/linux/mtd/rawnand.h:774: warning: Function parameter or member 'buf.in' not described in 'nand_op_data_instr' include/linux/mtd/rawnand.h:774: warning: Function parameter or member 'buf.out' not described in 'nand_op_data_instr' include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.cmd' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.data' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:820: warning: Function parameter or member 'ctx.waitrdy' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:967: warning: Function parameter or member 'ctx' not described in 'nand_op_parser_pattern_elem' include/linux/mtd/rawnand.h:967: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_parser_pattern_elem' include/linux/mtd/rawnand.h:967: warning: Function parameter or member 'ctx.data' not described in 'nand_op_parser_pattern_elem' include/linux/mtd/rawnand.h:1281: warning: Function parameter or member 'manufacturer.desc' not described in 'nand_chip' include/linux/mtd/rawnand.h:1281: warning: Function parameter or member 'manufacturer.priv' not described in 'nand_chip' include/linux/regulator/driver.h:221: warning: Function parameter or member 'resume_early' not described in 'regulator_ops' drivers/regulator/core.c:4299: warning: Excess function parameter 'state' description in 'regulator_suspend_late' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
drivers/gpu/drm/drm_modes.c:1784: warning: Function parameter or member 'flags' not described in 'drm_mode_aspect_ratio_allowed'
drivers/gpu/drm/drm_modes.c:1784: warning: Excess function parameter 'flag' description in 'drm_mode_aspect_ratio_allowed'
drivers/gpu/drm/drm_modes.c:1804: warning: Function parameter or member 'flags' not described in 'drm_mode_handle_aspect_ratio'
drivers/gpu/drm/drm_modes.c:1804: warning: Excess function parameter 'flag' description in 'drm_mode_handle_aspect_ratio' include/drm/drm_vblank.h:60: warning: Incorrect use of kernel-doc format: Documentation Makefile scripts source @event.base: DRM event base class. include/drm/drm_vblank.h:65: warning: Incorrect use of kernel-doc format: Documentation Makefile scripts source @event.vbl: include/drm/drm_vblank.h:75: warning: Incorrect use of kernel-doc format: Documentation Makefile scripts source @event.seq: Event payload for the MODE_QUEUEU_SEQUENCE IOCTL. include/drm/drm_vblank.h:79: warning: Function parameter or member 'event.base' not described in 'drm_pending_vblank_event' include/drm/drm_vblank.h:79: warning: Function parameter or member 'event.vbl' not described in 'drm_pending_vblank_event' include/drm/drm_vblank.h:79: warning: Function parameter or member 'event.seq' not described in 'drm_pending_vblank_event' drivers/gpu/drm/drm_vblank.c:1252: warning: Function parameter or member 'dev' not described in 'drm_vblank_restore' drivers/gpu/drm/drm_vblank.c:1252: warning: Function parameter or member 'pipe' not described in 'drm_vblank_restore' drivers/gpu/drm/drm_vblank.c:1295: warning: Function parameter or member 'crtc' not described in 'drm_crtc_vblank_restore' drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'attach' not described in 'drm_gem_unmap_dma_buf' drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'sgt' not described in 'drm_gem_unmap_dma_buf' drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'dir' not described in 'drm_gem_unmap_dma_buf' drivers/gpu/drm/drm_prime.c:438: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kmap_atomic' drivers/gpu/drm/drm_prime.c:438: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kmap_atomic' drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kunmap_atomic' drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kunmap_atomic' drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'addr' not described in 'drm_gem_dmabuf_kunmap_atomic' drivers/gpu/drm/drm_prime.c:461: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kmap' drivers/gpu/drm/drm_prime.c:461: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kmap' drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kunmap' drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kunmap' drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'addr' not described in 'drm_gem_dmabuf_kunmap' drivers/gpu/drm/i915/intel_dpio_phy.c:150: warning: Incorrect use of kernel-doc format: Documentation Makefile scripts source @channel.port: which port maps to this channel. drivers/gpu/drm/tve200/tve200_drv.c:1: warning: no structured comments found include/linux/skbuff.h:846: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'skb_mstamp' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'head_frag' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member '__unused' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'pfmemalloc' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'encapsulation' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'csum_valid' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'csum_level' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff' include/linux/skbuff.h:846: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff' include/net/sock.h:234: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_portpair' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_cookie' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_listener' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common' include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock' include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.len' not described in 'sock' include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.head' not described in 'sock' include/net/sock.h:487: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock' include/net/sock.h:487: warning: Function parameter or member 'sk_wq_raw' not described in 'sock' include/net/sock.h:487: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock' include/linux/netdevice.h:1940: warning: Function parameter or member 'adj_list.upper' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'adj_list.lower' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'gso_partial_features' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'switchdev_ops' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'name_assign_type' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'mpls_ptr' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'xdp_prog' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device' include/linux/netdevice.h:1940: warning: Function parameter or member 'qdisc_hash' not described in 'net_device' include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state' include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state' include/linux/rcupdate.h:570: ERROR: Unexpected indentation. include/linux/rcupdate.h:574: ERROR: Unexpected indentation. include/linux/rcupdate.h:578: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/rcupdate.h:580: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/rcupdate.h:580: WARNING: Inline literal start-string without end-string. kernel/time/timer.c:1259: ERROR: Unexpected indentation. kernel/time/timer.c:1261: ERROR: Unexpected indentation. kernel/time/timer.c:1262: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/wait.h:110: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/wait.h:113: ERROR: Unexpected indentation. include/linux/wait.h:115: WARNING: Block quote ends without a blank line; unexpected unindent. kernel/time/hrtimer.c:1113: WARNING: Block quote ends without a blank line; unexpected unindent. kernel/signal.c:327: WARNING: Inline literal start-string without end-string. drivers/video/fbdev/core/modedb.c:647: WARNING: Inline strong start-string without end-string. drivers/video/fbdev/core/modedb.c:647: WARNING: Inline strong start-string without end-string. drivers/video/fbdev/core/modedb.c:647: WARNING: Inline strong start-string without end-string. drivers/video/fbdev/core/modedb.c:647: WARNING: Inline strong start-string without end-string. include/linux/iio/iio.h:191: ERROR: Unexpected indentation. include/linux/iio/iio.h:192: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/iio/iio.h:198: WARNING: Definition list ends without a blank line; unexpected unindent. drivers/ata/libata-core.c:5920: ERROR: Unknown target name: "hw". drivers/message/fusion/mptbase.c:5052: WARNING: Definition list ends without a blank line; unexpected unindent. drivers/tty/serial/serial_core.c:1901: WARNING: Definition list ends without a blank line; unexpected unindent.
vim +1784 drivers/gpu/drm/drm_modes.c
1764 1765 /** 1766 * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information 1767 * is expected from the user-mode. 1768 * 1769 * If the user has set aspect-ratio cap, then the flag of the user-mode is 1770 * allowed to contain aspect-ratio value. 1771 * If the user does not set aspect-ratio cap, then the only value allowed in the 1772 * flags bits is aspect-ratio NONE. 1773 * 1774 * @file_priv: file private structure to get the user capabilities 1775 * @flag: 32 bit flag that contains the aspect-ratio information. 1776 * 1777 * Returns: 1778 * true if the aspect-ratio info is allowed in the user-mode flags. 1779 * false, otherwise. 1780 */ 1781 bool 1782 drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv, uint32_t flags) 1783 {
1784 return file_priv->aspect_ratio_allowed ||
1785 (flags & DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE; 1786 } 1787 EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed); 1788 1789 /** 1790 * drm_mode_handle_aspect_ratio - handles the aspect-ratio bits in the user-mode 1791 * flags. 1792 * 1793 * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio 1794 * bits in the user-mode flag, if aspect-ratio info is not allowed. 1795 * 1796 * @file_priv: file private structure to get the user capabilities. 1797 * @flag: 32 bit flag that is to be modified, in case the aspect 1798 * ratio info is not allowed. 1799 * 1800 */ 1801 void 1802 drm_mode_handle_aspect_ratio(const struct drm_file *file_priv, uint32_t *flags) 1803 {
1804 if (!drm_mode_aspect_ratio_allowed(file_priv, *flags))
--- 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, 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
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. --- drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_modes.c | 44 ++++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_property.c | 5 +++++ include/drm/drm_modes.h | 4 ++++ include/drm/drm_property.h | 2 ++ 5 files changed, 56 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..4430294 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1761,3 +1761,47 @@ 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 + * @flags: 32 bit flag that contains 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, uint32_t flags) +{ + return file_priv->aspect_ratio_allowed || + (flags & DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE; +} +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed); + +/** + * drm_mode_handle_aspect_ratio - handles 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 flag, if aspect-ratio info is not allowed. + * + * @file_priv: file private structure to get the user capabilities. + * @flags: 32 bit flag that is to be modified, in case the aspect + * ratio info is not allowed. + * + */ +void +drm_mode_handle_aspect_ratio(const struct drm_file *file_priv, uint32_t *flags) +{ + if (!drm_mode_aspect_ratio_allowed(file_priv, *flags)) + *flags &= ~DRM_MODE_FLAG_PIC_AR_MASK; +} +EXPORT_SYMBOL(drm_mode_handle_aspect_ratio); diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index c37ac41..0a9c879 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -757,6 +757,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_handle_aspect_ratio(file_priv, &mode->flags); + } } out_resp->length = blob->length; unref: diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 2f78b7e..51d1188 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, + uint32_t flags); +void drm_mode_handle_aspect_ratio(const struct drm_file *file_priv, + uint32_t *flags);
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 8a522b4..95e6e32 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[]; };
On Fri, Mar 09, 2018 at 11:07:14AM +0530, Nautiyal, Ankit K wrote:
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
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.
drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_modes.c | 44 ++++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_property.c | 5 +++++ include/drm/drm_modes.h | 4 ++++ include/drm/drm_property.h | 2 ++ 5 files changed, 56 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;diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index a48672c..4430294 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1761,3 +1761,47 @@ 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
- @flags: 32 bit flag that contains 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, uint32_t flags)
I think we should pass the entire umode to these functions. That way the compiler will make sure we're passing in the correct thing instead of some other mode flags or random integers.
+{
- return file_priv->aspect_ratio_allowed ||
(flags & DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;+} +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed);
+/**
- drm_mode_handle_aspect_ratio - handles 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 flag, if aspect-ratio info is not allowed.
- @file_priv: file private structure to get the user capabilities.
- @flags: 32 bit flag that is to be modified, in case the aspect
- ratio info is not allowed.
- */
+void +drm_mode_handle_aspect_ratio(const struct drm_file *file_priv, uint32_t *flags) +{
- if (!drm_mode_aspect_ratio_allowed(file_priv, *flags))
*flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;+} +EXPORT_SYMBOL(drm_mode_handle_aspect_ratio);
The function name could be more descriptive. drm_mode_filter_aspect_ratio_flags() or someting similar perhaps?
Otherwise I think this is looking good.
IIRC I did have a few more patches in my last inforame series to make sure we don't send invalid aspect ratio values in the infoframe. Unless something has changed I believe we still need those as well. Or am I wrong?
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index c37ac41..0a9c879 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -757,6 +757,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_handle_aspect_ratio(file_priv, &mode->flags);}unref: diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 2f78b7e..51d1188 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,
uint32_t flags);+void drm_mode_handle_aspect_ratio(const struct drm_file *file_priv,
uint32_t *flags);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 8a522b4..95e6e32 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[];
};
2.7.4
On 3/9/2018 6:40 PM, Ville Syrjälä wrote:
On Fri, Mar 09, 2018 at 11:07:14AM +0530, Nautiyal, Ankit K wrote:
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
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.
drivers/gpu/drm/drm_atomic.c | 1 + drivers/gpu/drm/drm_modes.c | 44 ++++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_property.c | 5 +++++ include/drm/drm_modes.h | 4 ++++ include/drm/drm_property.h | 2 ++ 5 files changed, 56 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;diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index a48672c..4430294 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1761,3 +1761,47 @@ 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
- @flags: 32 bit flag that contains 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, uint32_t flags)
I think we should pass the entire umode to these functions. That way the compiler will make sure we're passing in the correct thing instead of some other mode flags or random integers.
Agreed, that makes sense. Will change parameter to the umode.
+{
- return file_priv->aspect_ratio_allowed ||
(flags & DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;+} +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed);
+/**
- drm_mode_handle_aspect_ratio - handles 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 flag, if aspect-ratio info is not allowed.
- @file_priv: file private structure to get the user capabilities.
- @flags: 32 bit flag that is to be modified, in case the aspect
- ratio info is not allowed.
- */
+void +drm_mode_handle_aspect_ratio(const struct drm_file *file_priv, uint32_t *flags) +{
- if (!drm_mode_aspect_ratio_allowed(file_priv, *flags))
*flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;+} +EXPORT_SYMBOL(drm_mode_handle_aspect_ratio);
The function name could be more descriptive. drm_mode_filter_aspect_ratio_flags() or someting similar perhaps?
Point taken. I was a little worried about the length of the function name. But having a meaningful name is important. Will change this also, in the next patch set.
Otherwise I think this is looking good.
IIRC I did have a few more patches in my last inforame series to make sure we don't send invalid aspect ratio values in the infoframe. Unless something has changed I believe we still need those as well. Or am I wrong?
You are right, There are two patches: https://patchwork.freedesktop.org/patch/188049/ and https://patchwork.freedesktop.org/patch/188051/ from the info-frame clean up series, both of which have already received r-b.
These patches need a little change, as they reject picture aspect ratio
16:9
Since we are adding support of 64:27 and 256:135, then we need to update the reject condition. I can make the required changes and add these two patches as part of this series.
What do you suggest? Is it fine to make these changes?
Regards, Ankit
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index c37ac41..0a9c879 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -757,6 +757,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_handle_aspect_ratio(file_priv, &mode->flags);}diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 2f78b7e..51d1188 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,
uint32_t flags);+void drm_mode_handle_aspect_ratio(const struct drm_file *file_priv,
uint32_t *flags);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 8a522b4..95e6e32 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[]; };
-- 2.7.4
On Mon, Mar 12, 2018 at 01:32:48PM +0530, Nautiyal, Ankit K wrote:
On 3/9/2018 6:40 PM, Ville Syrjälä wrote:
IIRC I did have a few more patches in my last inforame series to make sure we don't send invalid aspect ratio values in the infoframe. Unless something has changed I believe we still need those as well. Or am I wrong?
You are right, There are two patches: https://patchwork.freedesktop.org/patch/188049/ and https://patchwork.freedesktop.org/patch/188051/ from the info-frame clean up series, both of which have already received r-b.
These patches need a little change, as they reject picture aspect ratio
16:9
Since we are adding support of 64:27 and 256:135, then we need to update the reject condition.
IIRC the whole point was that the inforframe can't convey 64:27 or 256:135. Thus the >16:9 cheeck still looks correct to me.
I can make the required changes and add these two patches as part of this series.
What do you suggest? Is it fine to make these changes?
Regards, Ankit
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index c37ac41..0a9c879 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -757,6 +757,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_handle_aspect_ratio(file_priv, &mode->flags);}diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 2f78b7e..51d1188 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,
uint32_t flags);+void drm_mode_handle_aspect_ratio(const struct drm_file *file_priv,
uint32_t *flags);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 8a522b4..95e6e32 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[]; };
-- 2.7.4
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. --- 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 86b483e..cd5fd13 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->flags)) { + 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) { @@ -1918,7 +1932,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; @@ -1952,7 +1967,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: { @@ -2341,7 +2356,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 ae3cbfe..781ebd6 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;
@@ -2749,7 +2749,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;
@@ -2930,7 +2930,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 353e24f..a500821 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_handle_aspect_ratio(file_priv, &crtc_resp->mode.flags); 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.flags)) { + 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 af00f42..7c3338f 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -186,7 +186,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 2c711a2..128bd07 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. --- 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..4561239 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_handle_aspect_ratio(file_priv, &u_mode.flags); 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. --- 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 219890f..79b2a24 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 --- 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 79b2a24..c3d9a228 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 2c57579..93c27cd 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 -
Ville Syrjälä