[PATCH v11 00/11] Aspect ratio support in DRM layer

[PATCH v11 00/11] Aspect ratio support in DRM layer

[Nautiyal, Ankit K]

From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

This patch series is a re-attempt to enable aspect ratio support in
DRM layer. Currently the aspect ratio information gets lost in translation
during a user->kernel mode or vice versa.

The old patch series (https://pw-emeril.freedesktop.org/series/10850/) had
4 patches, out of which 2 patches were reverted due to lack of drm client
protection while loading the aspect information.

This patch series also includes 5 patches from Ville Syrjälä's series for
'Info-frame cleanup and fixes':
https://patchwork.freedesktop.org/series/33730/ which fixes the mode
matching mechanism via flags, and also ensures that no bogus aspect-ratios
are sent in the AVI infoframes.

This patch series, adds a DRM client option for aspect ratio, and loads
aspect ratio flags, only when the client sets this cap. 

To test this patch, the testdiplay IGT test is modified to have an option
to do a modeset with only aspect ratio modes.
Also, there is a userspace implementation in Wayland/weston layer:
https://patchwork.freedesktop.org/patch/188125/
(Which is already ACK'ed by wayland community.)

This, helps us in passing HDMI compliance test cases like 7-27, where the
test equipment applies a CEA mode, and expects the exact VIC in the AVI
infoframes.

Ankit Nautiyal (4):
  drm: Add DRM client cap for aspect-ratio
  drm: Add helper functions to handle aspect-ratio flag bits
  drm: Handle aspect ratio info in legacy and atomic modeset paths
  drm: Expose modes with aspect ratio, only if requested

Sharma, Shashank (2):
  drm: Add aspect ratio parsing in DRM layer
  drm: Add and handle new aspect ratios in DRM layer

Ville Syrjälä (5):
  drm/modes: Introduce drm_mode_match()
  drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
  drm/edid: Fix cea mode aspect ratio handling
  drm/edid: Don't send bogus aspect ratios in AVI infoframes
  video/hdmi: Reject illegal picture aspect ratios

 drivers/gpu/drm/drm_atomic.c        |  34 ++++--
 drivers/gpu/drm/drm_atomic_helper.c |   6 +-
 drivers/gpu/drm/drm_connector.c     |  56 +++++++--
 drivers/gpu/drm/drm_crtc.c          |   8 ++
 drivers/gpu/drm/drm_crtc_internal.h |   3 +-
 drivers/gpu/drm/drm_edid.c          |  41 +++++--
 drivers/gpu/drm/drm_fb_helper.c     |  12 +-
 drivers/gpu/drm/drm_ioctl.c         |   9 ++
 drivers/gpu/drm/drm_mode_object.c   |   9 +-
 drivers/gpu/drm/drm_modes.c         | 226 +++++++++++++++++++++++++++++++-----
 drivers/video/hdmi.c                |   3 +
 include/drm/drm_atomic.h            |   5 +-
 include/drm/drm_file.h              |   8 ++
 include/drm/drm_modes.h             |  13 +++
 include/uapi/drm/drm.h              |   7 ++
 include/uapi/drm/drm_mode.h         |   6 +
 16 files changed, 377 insertions(+), 69 deletions(-)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v11 01/11] drm/modes: Introduce drm_mode_match()

[Nautiyal, Ankit K]

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 e82b61e..c395a24 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -939,17 +939,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;
@@ -957,15 +1008,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;
 
-	return drm_mode_equal_no_clocks(mode1, mode2);
+	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;
+
+	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);
 
@@ -980,13 +1064,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);
 
@@ -1004,21 +1088,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,
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v11 02/11] drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy

[Nautiyal, Ankit K]

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 134069f..c35d3bc 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3047,7 +3047,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;
 	}
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v11 03/11] drm/edid: Fix cea mode aspect ratio handling

[Nautiyal, Ankit K]

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 c35d3bc..29c88eb 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2930,11 +2930,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;
@@ -2948,7 +2952,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));
 	}
@@ -2965,11 +2969,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;
@@ -2983,7 +2991,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));
 	}
@@ -3030,6 +3038,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)
@@ -3047,7 +3056,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;
 	}
 
@@ -3064,6 +3073,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)
@@ -3079,7 +3089,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;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v11 04/11] drm/edid: Don't send bogus aspect ratios in AVI infoframes

[Nautiyal, Ankit K]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If the user mode would specify an aspect ratio other than 4:3 or 16:9
we now silently ignore it. Maybe a better apporoach is to return an
error? Let's try that.

Also we must be careful that we don't try to send illegal picture
aspect in the infoframe as it's only capable of signalling none,
4:3, and 16:9. Currently we're sending these bogus infoframes
whenever the cea mode specifies some other aspect ratio.

Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 29c88eb..d5757aa 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4840,6 +4840,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 					 const struct drm_display_mode *mode,
 					 bool is_hdmi2_sink)
 {
+	enum hdmi_picture_aspect picture_aspect;
 	int err;
 
 	if (!frame || !mode)
@@ -4882,13 +4883,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 	 * Populate picture aspect ratio from either
 	 * user input (if specified) or from the CEA mode list.
 	 */
-	if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
-		mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
-		frame->picture_aspect = mode->picture_aspect_ratio;
-	else if (frame->video_code > 0)
-		frame->picture_aspect = drm_get_cea_aspect_ratio(
-						frame->video_code);
+	picture_aspect = mode->picture_aspect_ratio;
+	if (picture_aspect == HDMI_PICTURE_ASPECT_NONE)
+		picture_aspect = drm_get_cea_aspect_ratio(frame->video_code);
 
+	/*
+	 * The infoframe can't convey anything but none, 4:3
+	 * and 16:9, so if the user has asked for anything else
+	 * we can only satisfy it by specifying the right VIC.
+	 */
+	if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) {
+		if (picture_aspect !=
+		    drm_get_cea_aspect_ratio(frame->video_code))
+			return -EINVAL;
+		picture_aspect = HDMI_PICTURE_ASPECT_NONE;
+	}
+
+	frame->picture_aspect = picture_aspect;
 	frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
 	frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
 
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v11 05/11] video/hdmi: Reject illegal picture aspect ratios

[Nautiyal, Ankit K]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

AVI infoframe can only carry none, 4:3, or 16:9 picture aspect
ratios. Return an error if the user asked for something different.

Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: "Lin, Jia" <lin.a.jia@intel.com>
Cc: Akashdeep Sharma <akashdeep.sharma@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: linux-media@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/video/hdmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 111a0ab..38716eb5 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -93,6 +93,9 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer,
 	if (size < length)
 		return -ENOSPC;
 
+	if (frame->picture_aspect > HDMI_PICTURE_ASPECT_16_9)
+		return -EINVAL;
+
 	memset(buffer, 0, size);
 
 	ptr[0] = frame->type;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v11 06/11] drm: Add DRM client cap for aspect-ratio

[Nautiyal, Ankit K]

From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

To enable aspect-ratio support in DRM, blindly exposing the aspect
ratio information along with mode, can break things in existing
user-spaces which have no intention or support to use this aspect
ratio information.

To avoid this, a new drm client cap is required to enable a
user-space to advertise if it supports modes with aspect-ratio. Based
on this cap value, the kernel will take a call on exposing the aspect
ratio info in modes or not.

This patch adds the client cap for aspect-ratio.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

V3: rebase
V4: As suggested by Marteen Lankhorst modified the commit message
    explaining the need to use the DRM cap for aspect-ratio. Also,
    tweaked the comment lines in the code for better understanding and
    clarity, as recommended by Shashank Sharma.
V5: rebase
V6: rebase
V7: rebase
V8: rebase
V9: rebase
V10: added comment explaining that no userspace breaks on aspect-ratio
     mode bits.

Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 9 +++++++++
 include/drm/drm_file.h      | 8 ++++++++
 include/uapi/drm/drm.h      | 7 +++++++
 3 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index af78291..39c8eab 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -325,6 +325,15 @@ 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;
+	/*
+	 * No Atomic userspace blows up on aspect ratio mode bits. Checked in
+	 * wayland/weston, xserver, and hardware-composer modeset paths.
+	 */
+		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;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v11 07/11] drm: Add helper functions to handle aspect-ratio flag bits

[Nautiyal, Ankit K]

From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

This patch adds helper functions for determining if aspect-ratio is
expected in user-mode and for allowing/disallowing the aspect-ratio,
if its not expected.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/drm_modes.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_modes.h     |  4 ++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index c395a24..d6133e8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1759,3 +1759,50 @@ bool drm_mode_is_420(const struct drm_display_info *display,
 		drm_mode_is_420_also(display, mode);
 }
 EXPORT_SYMBOL(drm_mode_is_420);
+
+/**
+ * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information
+ * is expected from the user-mode.
+ *
+ * If the user has set aspect-ratio cap, then the flag of the user-mode is
+ * allowed to contain aspect-ratio value.
+ * If the user does not set aspect-ratio cap, then the only value allowed in the
+ * flags bits is aspect-ratio NONE.
+ *
+ * @file_priv: file private structure to get the user capabilities
+ * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio
+ * information.
+ *
+ * Returns:
+ * true if the aspect-ratio info is allowed in the user-mode flags.
+ * false, otherwise.
+ */
+bool
+drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
+			      struct drm_mode_modeinfo *umode)
+{
+	return file_priv->aspect_ratio_allowed || (umode->flags &
+		DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;
+}
+EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed);
+
+/**
+ * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the
+ * user-mode flags.
+ *
+ * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio
+ * bits in the user-mode flags, if aspect-ratio info is not allowed.
+ *
+ * @file_priv: file private structure to get the user capabilities.
+ * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to
+ * be filtered.
+ *
+ */
+void
+drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
+				   struct drm_mode_modeinfo *umode)
+{
+	if (!drm_mode_aspect_ratio_allowed(file_priv, umode))
+		umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
+}
+EXPORT_SYMBOL(drm_mode_filter_aspect_ratio_flags);
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 2f78b7e..e0b060d 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display,
 			  const struct drm_display_mode *mode);
 bool drm_mode_is_420(const struct drm_display_info *display,
 		     const struct drm_display_mode *mode);
+bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
+				   struct drm_mode_modeinfo *umode);
+void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
+					struct drm_mode_modeinfo *umode);
 
 struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
 				      int hdisplay, int vdisplay, int vrefresh,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v11 08/11] drm: Handle aspect ratio info in legacy and atomic modeset paths

[Nautiyal, Ankit K]

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.

Note: In case, a user-space asks for a video-mode blob, from the
getblob ioctl, the aspect-ratio bits in the video-mode blob are passed
to the user as it is, without any filtering. However, no such case is
present in most of the atomic user-spaces. Currently atomic path of
Xserver, Wayland/weston, Hardware-Composer are checked, and none of
them are using getblob ioctl to get the video-mode blob.

This patch:
1. passes the file_priv structure from the drm_mode_atomic_ioctl till
   the drm_mode_crtc_set_mode_prop, to get the user capability.
2. rejects the modes with aspect-ratio info, during modeset, if the
   user does not support aspect ratio.
3. does not load the aspect-ratio info in user-mode structure, if
   aspect ratio is not supported.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

V3: Addressed review comments from Ville:
    Do not corrupt the current crtc state by updating aspect-ratio on
    the fly.
V4: rebase
V5: As suggested by Ville, rejected the modeset calls for modes with
    aspect ratio, if the user does not set aspect-ratio cap.
V6: Used the helper functions for determining if aspect-ratio is
    expected in the user-mode.
V7: rebase
V8: rebase
V9: rebase
v10: Modified the commit-message
---
 drivers/gpu/drm/drm_atomic.c        | 34 +++++++++++++++++++++++++---------
 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(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3d9ae05..5acf49d 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,9 +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,
-					   blob->data))
+		struct drm_mode_modeinfo *u_mode;
+
+		if (blob->length != sizeof(struct drm_mode_modeinfo))
+			return -EINVAL;
+
+		u_mode = (struct drm_mode_modeinfo *) blob->data;
+		if (!drm_mode_aspect_ratio_allowed(file_priv,
+						   u_mode)) {
+			DRM_DEBUG_ATOMIC("Unexpected aspect-ratio flag bits\n");
+			return -EINVAL;
+		}
+
+		if (drm_mode_convert_umode(state->crtc->dev, &state->mode,
+					   (const struct drm_mode_modeinfo *)
+					   u_mode))
 			return -EINVAL;
 
 		state->mode_blob = drm_property_blob_get(blob);
@@ -471,6 +485,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
@@ -482,7 +497,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;
@@ -494,7 +509,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);
-		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) {
@@ -1965,7 +1980,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;
@@ -1999,7 +2015,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: {
@@ -2388,7 +2404,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 9cb2209..d27db14 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;
 
@@ -2773,7 +2773,7 @@ static int update_output_state(struct drm_atomic_state *state,
 
 		if (!new_crtc_state->connector_mask) {
 			ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
-								NULL);
+								NULL, NULL);
 			if (ret < 0)
 				return ret;
 
@@ -2932,7 +2932,7 @@ static 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 a231dd5..98323f4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -449,6 +449,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
 			crtc_resp->mode_valid = 0;
 		}
 	}
+	drm_mode_filter_aspect_ratio_flags(file_priv, &crtc_resp->mode);
 	drm_modeset_unlock(&crtc->mutex);
 
 	return 0;
@@ -628,6 +629,13 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 			ret = -ENOMEM;
 			goto out;
 		}
+		if (!drm_mode_aspect_ratio_allowed(file_priv,
+						   &crtc_req->mode)) {
+			DRM_DEBUG_KMS("Unexpected aspect-ratio flag bits\n");
+			ret = -EINVAL;
+			goto out;
+		}
+
 
 		ret = drm_mode_convert_umode(dev, mode, &crtc_req->mode);
 		if (ret) {
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 3c2b828..7e9f8f8 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -188,7 +188,8 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state,
 int drm_atomic_set_property(struct drm_atomic_state *state,
 			    struct drm_mode_object *obj,
 			    struct drm_property *prop,
-			    uint64_t prop_value);
+			    uint64_t prop_value,
+			    struct drm_file *file_priv);
 int drm_atomic_get_property(struct drm_mode_object *obj,
 			    struct drm_property *property, uint64_t *val);
 int drm_mode_atomic_ioctl(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index ce4d2fb..1c3d498 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -452,7 +452,8 @@ static int set_property_legacy(struct drm_mode_object *obj,
 
 static int set_property_atomic(struct drm_mode_object *obj,
 			       struct drm_property *prop,
-			       uint64_t prop_value)
+			       uint64_t prop_value,
+			       struct drm_file *file_priv)
 {
 	struct drm_device *dev = prop->dev;
 	struct drm_atomic_state *state;
@@ -476,7 +477,8 @@ static int set_property_atomic(struct drm_mode_object *obj,
 						       obj_to_connector(obj),
 						       prop_value);
 	} else {
-		ret = drm_atomic_set_property(state, obj, prop, prop_value);
+		ret = drm_atomic_set_property(state, obj, prop, prop_value,
+					      file_priv);
 		if (ret)
 			goto out;
 		ret = drm_atomic_commit(state);
@@ -519,7 +521,8 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
 		goto out_unref;
 
 	if (drm_drv_uses_atomic_modeset(property->dev))
-		ret = set_property_atomic(arg_obj, property, arg->value);
+		ret = set_property_atomic(arg_obj, property, arg->value,
+					  file_priv);
 	else
 		ret = set_property_legacy(arg_obj, property, arg->value);
 
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index a57a8aa..b7106f7 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -367,7 +367,7 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
 			  struct drm_crtc *crtc);
 int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		struct drm_crtc_state *state, struct drm_property *property,
-		uint64_t val);
+		uint64_t val, struct drm_file *file_priv);
 struct drm_plane_state * __must_check
 drm_atomic_get_plane_state(struct drm_atomic_state *state,
 			   struct drm_plane *plane);
@@ -583,7 +583,8 @@ drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
 			     const struct drm_display_mode *mode);
 int __must_check
 drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
-				  struct drm_property_blob *blob);
+				  struct drm_property_blob *blob,
+				  struct drm_file *file_priv);
 int __must_check
 drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,
 			      struct drm_crtc *crtc);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v11 09/11] drm: Expose modes with aspect ratio, only if requested

[Nautiyal, Ankit K]

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, regardless 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. However if such a mode is unique in the list, it is
kept in the list, with aspect-ratio flags reset.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Jose Abreu <jose.abreu@synopsys.com>

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

V3: As suggested by Ville, modified the mechanism of pruning of modes
    with aspect-ratio, if the aspect-ratio is not supported. Instead
    of straight away pruning such a mode, the mode is retained with
    aspect ratio bits set to zero, provided it is unique.
V4: rebase
V5: Addressed review comments from Ville:
    -used a pointer to store last valid mode.
    -avoided, modifying of picture_aspect_ratio in kernel mode,
     instead only flags bits of user mode are reset (if aspect-ratio
     is not supported).
V6: As suggested by Ville, corrected the mode pruning logic and
    elaborated the mode pruning logic and the assumptions taken.
V7: rebase
V8: rebase
V9: rebase
V10: rebase
V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
     logic to correctly identify and prune modes with aspect-ratio,
     if aspect-ratio cap is not set.
---
 drivers/gpu/drm/drm_connector.c | 56 +++++++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b3cde89..865ee354 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1531,15 +1531,35 @@ 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 *modelist,
+			     const struct drm_file *file_priv)
 {
 	/*
 	 * If user-space hasn't configured the driver to expose the stereo 3D
 	 * modes, don't expose them.
 	 */
+	struct drm_display_mode *mode_itr;
+
 	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. However if such a mode
+	 * is unique, let it be exposed, but reset the aspect-ratio flags
+	 * while preparing the list of user-modes.
+	 */
+	if (!file_priv->aspect_ratio_allowed &&
+	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
+		list_for_each_entry(mode_itr, &modelist->head, head)
+			if (drm_mode_match(mode_itr, mode,
+					   DRM_MODE_MATCH_TIMINGS |
+					   DRM_MODE_MATCH_CLOCK |
+					   DRM_MODE_MATCH_FLAGS |
+					   DRM_MODE_MATCH_3D_FLAGS))
+				return false;
+	}
 
 	return true;
 }
@@ -1550,7 +1570,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	struct drm_mode_get_connector *out_resp = data;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
-	struct drm_display_mode *mode;
+	struct drm_display_mode *mode, *tmp, modelist;
 	int mode_count = 0;
 	int encoders_count = 0;
 	int ret = 0;
@@ -1605,23 +1625,37 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	out_resp->subpixel = connector->display_info.subpixel_order;
 	out_resp->connection = connector->status;
 
+	INIT_LIST_HEAD(&modelist.head);
+
 	/* delayed so we get modes regardless of pre-fill_modes state */
 	list_for_each_entry(mode, &connector->modes, head)
-		if (drm_mode_expose_to_userspace(mode, file_priv))
+		if (drm_mode_expose_to_userspace(mode, &modelist,
+						 file_priv)) {
+			struct drm_display_mode *tmp_mode;
+
+			tmp_mode = drm_mode_duplicate(dev, mode);
+			list_add_tail(&tmp_mode->head, &modelist.head);
 			mode_count++;
+		}
 
 	/*
 	 * This ioctl is called twice, once to determine how much space is
 	 * needed, and the 2nd time to fill it.
+	 * The modes that need to be exposed to the user are maintained in the
+	 * 'modelist'. When the ioctl is called first time to determine the,
+	 * space, the modelist gets filled, to find the no. of modes. In the
+	 * 2nd time, the user modes are filled, one by one from the modelist.
 	 */
 	if ((out_resp->count_modes >= mode_count) && mode_count) {
 		copied = 0;
 		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
-		list_for_each_entry(mode, &connector->modes, head) {
-			if (!drm_mode_expose_to_userspace(mode, file_priv))
-				continue;
-
+		list_for_each_entry(mode, &modelist.head, head) {
 			drm_mode_convert_to_umode(&u_mode, mode);
+			/*
+			 * Reset aspect ratio flags of user-mode, if modes with
+			 * aspect-ratio are not supported.
+			 */
+			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
 			if (copy_to_user(mode_ptr + copied,
 					 &u_mode, sizeof(u_mode))) {
 				ret = -EFAULT;
@@ -1651,6 +1685,12 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 
 out:
+	list_for_each_entry_safe(mode, tmp, &modelist.head, head) {
+		list_del(&mode->head);
+		drm_mode_destroy(dev, mode);
+	}
+	list_del(&modelist.head);
+
 	drm_connector_put(connector);
 
 	return ret;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v11 10/11] drm: Add aspect ratio parsing in DRM layer

[Nautiyal, Ankit K]

From: "Sharma, Shashank" <shashank.sharma@intel.com>

Current DRM layer functions don't parse aspect ratio information
while converting a user mode->kernel mode or vice versa. This
causes modeset to pick mode with wrong aspect ratio, eventually
causing failures in HDMI compliance test cases, due to wrong VIC.

This patch adds aspect ratio information in DRM's mode conversion
and mode comparision functions, to make sure kernel picks mode
with right aspect ratio (as per the VIC).

Background:
This patch was once reviewed and merged, and later reverted due to
lack of DRM cap protection. This is a re-spin of this patch, this
time with DRM cap protection, to avoid aspect ratio information, when
the client doesn't request for it.

Review link: https://pw-emeril.freedesktop.org/patch/104068/
Background discussion: https://patchwork.kernel.org/patch/9379057/

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Lin, Jia <lin.a.jia@intel.com>
Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
Reviewed-by: Jim Bride <jim.bride@linux.intel.com> (V2)
Reviewed-by: Jose Abreu <Jose.Abreu@synopsys.com> (V4)

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

V3: modified the aspect-ratio check in drm_mode_equal as per new flags
    provided by Ville. https://patchwork.freedesktop.org/patch/188043/
V4: rebase
V5: rebase
V6: As recommended by Ville, avoided matching of aspect-ratio in
    drm_fb_helper, while trying to find a common mode among connectors
    for the target clone mode.
V7: rebase
V8: rebase
V9: rebase
V10: rebase
---
 drivers/gpu/drm/drm_fb_helper.c | 12 ++++++++++--
 drivers/gpu/drm/drm_modes.c     | 35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0646b10..2ee1eaa 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 d6133e8..454f2ff 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1049,7 +1049,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);
 
@@ -1647,6 +1648,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;
 }
@@ -1693,6 +1708,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)
 		return -EINVAL;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v11 11/11] drm: Add and handle new aspect ratios in DRM layer

[Nautiyal, Ankit K]

From: "Sharma, Shashank" <shashank.sharma@intel.com>

HDMI 2.0/CEA-861-F introduces two new aspect ratios:
- 64:27
- 256:135

This patch:
-  Adds new DRM flags for to represent these new aspect ratios.
-  Adds new cases to handle these aspect ratios while converting
from user->kernel mode or vise versa.

This patch was once reviewed and merged, and later reverted due
to lack of DRM client protection, while adding aspect ratio bits
in user modes. This is a re-spin of the series, with DRM client
cap protection.

The previous series can be found here:
https://pw-emeril.freedesktop.org/series/10850/

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org> (V2)
Reviewed-by: Jose Abreu <Jose.Abreu@synopsys.com> (V2)

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

V3: rebase
V4: rebase
V5: corrected the macro name for an aspect ratio, in a switch case.
V6: rebase
V7: rebase
V8: rebase
V9: rebase
V10: 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 454f2ff..21cc84b 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1656,6 +1656,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;
@@ -1721,6 +1727,12 @@ int drm_mode_convert_umode(struct drm_device *dev,
 	case DRM_MODE_FLAG_PIC_AR_16_9:
 		out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9;
 		break;
+	case DRM_MODE_FLAG_PIC_AR_64_27:
+		out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_64_27;
+		break;
+	case DRM_MODE_FLAG_PIC_AR_256_135:
+		out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_256_135;
+		break;
 	default:
 		out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 		break;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 50bcf42..4b3a1bb 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -93,6 +93,8 @@ extern "C" {
 #define DRM_MODE_PICTURE_ASPECT_NONE		0
 #define DRM_MODE_PICTURE_ASPECT_4_3		1
 #define DRM_MODE_PICTURE_ASPECT_16_9		2
+#define DRM_MODE_PICTURE_ASPECT_64_27		3
+#define DRM_MODE_PICTURE_ASPECT_256_135		4
 
 /* Aspect ratio flag bitmask (4 bits 22:19) */
 #define DRM_MODE_FLAG_PIC_AR_MASK		(0x0F<<19)
@@ -102,6 +104,10 @@ extern "C" {
 			(DRM_MODE_PICTURE_ASPECT_4_3<<19)
 #define  DRM_MODE_FLAG_PIC_AR_16_9 \
 			(DRM_MODE_PICTURE_ASPECT_16_9<<19)
+#define  DRM_MODE_FLAG_PIC_AR_64_27 \
+			(DRM_MODE_PICTURE_ASPECT_64_27<<19)
+#define  DRM_MODE_FLAG_PIC_AR_256_135 \
+			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
 
 #define  DRM_MODE_FLAG_ALL	(DRM_MODE_FLAG_PHSYNC |		\
 				 DRM_MODE_FLAG_NHSYNC |		\
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v11 06/11] drm: Add DRM client cap for aspect-ratio

[Ville Syrjälä]

On Fri, Apr 20, 2018 at 07:01:46PM +0530, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> To enable aspect-ratio support in DRM, blindly exposing the aspect
> ratio information along with mode, can break things in existing
> user-spaces which have no intention or support to use this aspect
> ratio information.
> 
> To avoid this, a new drm client cap is required to enable a
> user-space to advertise if it supports modes with aspect-ratio. Based
> on this cap value, the kernel will take a call on exposing the aspect
> ratio info in modes or not.
> 
> This patch adds the client cap for aspect-ratio.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> V3: rebase
> V4: As suggested by Marteen Lankhorst modified the commit message
>     explaining the need to use the DRM cap for aspect-ratio. Also,
>     tweaked the comment lines in the code for better understanding and
>     clarity, as recommended by Shashank Sharma.
> V5: rebase
> V6: rebase
> V7: rebase
> V8: rebase
> V9: rebase
> V10: added comment explaining that no userspace breaks on aspect-ratio
>      mode bits.
> 
> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 9 +++++++++
>  include/drm/drm_file.h      | 8 ++++++++
>  include/uapi/drm/drm.h      | 7 +++++++
>  3 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index af78291..39c8eab 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -325,6 +325,15 @@ 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;
> +	/*
> +	 * No Atomic userspace blows up on aspect ratio mode bits. Checked in
> +	 * wayland/weston, xserver, and hardware-composer modeset paths.
> +	 */

Bogus indentation.

Also where's the aspect_ratio_allowed handling for the atomic cap?
Or did we decide against it after all?

> +		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;
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v11 07/11] drm: Add helper functions to handle aspect-ratio flag bits

[Ville Syrjälä]

On Fri, Apr 20, 2018 at 07:01:47PM +0530, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> This patch adds helper functions for determining if aspect-ratio is
> expected in user-mode and for allowing/disallowing the aspect-ratio,
> if its not expected.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/drm_modes.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_modes.h     |  4 ++++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index c395a24..d6133e8 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1759,3 +1759,50 @@ bool drm_mode_is_420(const struct drm_display_info *display,
>  		drm_mode_is_420_also(display, mode);
>  }
>  EXPORT_SYMBOL(drm_mode_is_420);
> +
> +/**
> + * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information
> + * is expected from the user-mode.
> + *
> + * If the user has set aspect-ratio cap, then the flag of the user-mode is
> + * allowed to contain aspect-ratio value.
> + * If the user does not set aspect-ratio cap, then the only value allowed in the
> + * flags bits is aspect-ratio NONE.
> + *
> + * @file_priv: file private structure to get the user capabilities
> + * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio
> + * information.
> + *
> + * Returns:
> + * true if the aspect-ratio info is allowed in the user-mode flags.
> + * false, otherwise.
> + */
> +bool
> +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
> +			      struct drm_mode_modeinfo *umode)
> +{
> +	return file_priv->aspect_ratio_allowed || (umode->flags &
> +		DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;

Odd line split here. Makes this a bit hard to read.
I would split after the ||

> +}
> +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed);

Do we actually need to export these? I don't think so.

But I might be wrong. It's a bit hard to see with the way
you split this patch with the actual users in a different patch.

> +
> +/**
> + * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the
> + * user-mode flags.
> + *
> + * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio
> + * bits in the user-mode flags, if aspect-ratio info is not allowed.
> + *
> + * @file_priv: file private structure to get the user capabilities.
> + * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to
> + * be filtered.
> + *
> + */
> +void
> +drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
> +				   struct drm_mode_modeinfo *umode)
> +{
> +	if (!drm_mode_aspect_ratio_allowed(file_priv, umode))
> +		umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
> +}
> +EXPORT_SYMBOL(drm_mode_filter_aspect_ratio_flags);
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 2f78b7e..e0b060d 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display,
>  			  const struct drm_display_mode *mode);
>  bool drm_mode_is_420(const struct drm_display_info *display,
>  		     const struct drm_display_mode *mode);
> +bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
> +				   struct drm_mode_modeinfo *umode);
> +void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
> +					struct drm_mode_modeinfo *umode);
>  
>  struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
>  				      int hdisplay, int vdisplay, int vrefresh,
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v11 09/11] drm: Expose modes with aspect ratio, only if requested

[Ville Syrjälä]

On Fri, Apr 20, 2018 at 07:01:49PM +0530, Nautiyal, Ankit K wrote:
> 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, regardless 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. However if such a mode is unique in the list, it is
> kept in the list, with aspect-ratio flags reset.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Jose Abreu <jose.abreu@synopsys.com>
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> V3: As suggested by Ville, modified the mechanism of pruning of modes
>     with aspect-ratio, if the aspect-ratio is not supported. Instead
>     of straight away pruning such a mode, the mode is retained with
>     aspect ratio bits set to zero, provided it is unique.
> V4: rebase
> V5: Addressed review comments from Ville:
>     -used a pointer to store last valid mode.
>     -avoided, modifying of picture_aspect_ratio in kernel mode,
>      instead only flags bits of user mode are reset (if aspect-ratio
>      is not supported).
> V6: As suggested by Ville, corrected the mode pruning logic and
>     elaborated the mode pruning logic and the assumptions taken.
> V7: rebase
> V8: rebase
> V9: rebase
> V10: rebase
> V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
>      logic to correctly identify and prune modes with aspect-ratio,
>      if aspect-ratio cap is not set.
> ---
>  drivers/gpu/drm/drm_connector.c | 56 +++++++++++++++++++++++++++++++++++------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b3cde89..865ee354 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1531,15 +1531,35 @@ 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 *modelist,
> +			     const struct drm_file *file_priv)
>  {
>  	/*
>  	 * If user-space hasn't configured the driver to expose the stereo 3D
>  	 * modes, don't expose them.
>  	 */
> +	struct drm_display_mode *mode_itr;
> +
>  	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. However if such a mode
> +	 * is unique, let it be exposed, but reset the aspect-ratio flags
> +	 * while preparing the list of user-modes.
> +	 */
> +	if (!file_priv->aspect_ratio_allowed &&
> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
> +		list_for_each_entry(mode_itr, &modelist->head, head)
> +			if (drm_mode_match(mode_itr, mode,
> +					   DRM_MODE_MATCH_TIMINGS |
> +					   DRM_MODE_MATCH_CLOCK |
> +					   DRM_MODE_MATCH_FLAGS |
> +					   DRM_MODE_MATCH_3D_FLAGS))
> +				return false;
> +	}
>  
>  	return true;
>  }
> @@ -1550,7 +1570,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	struct drm_mode_get_connector *out_resp = data;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> -	struct drm_display_mode *mode;
> +	struct drm_display_mode *mode, *tmp, modelist;
>  	int mode_count = 0;
>  	int encoders_count = 0;
>  	int ret = 0;
> @@ -1605,23 +1625,37 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	out_resp->subpixel = connector->display_info.subpixel_order;
>  	out_resp->connection = connector->status;
>  
> +	INIT_LIST_HEAD(&modelist.head);

Why are we using a struct drm_display_mode to get a simple list_head?

> +
>  	/* delayed so we get modes regardless of pre-fill_modes state */
>  	list_for_each_entry(mode, &connector->modes, head)
> -		if (drm_mode_expose_to_userspace(mode, file_priv))
> +		if (drm_mode_expose_to_userspace(mode, &modelist,
> +						 file_priv)) {
> +			struct drm_display_mode *tmp_mode;
> +
> +			tmp_mode = drm_mode_duplicate(dev, mode);

Duplicating every mode seems rather wasteful. I suppose we could
just add another list_head to struct drm_display_mode for this
purpose. An alternative could be to somehow mark each exposed mode
and just walk the original connector mode list.

> +			list_add_tail(&tmp_mode->head, &modelist.head);
>  			mode_count++;
> +		}
>  
>  	/*
>  	 * This ioctl is called twice, once to determine how much space is
>  	 * needed, and the 2nd time to fill it.
> +	 * The modes that need to be exposed to the user are maintained in the
> +	 * 'modelist'. When the ioctl is called first time to determine the,
> +	 * space, the modelist gets filled, to find the no. of modes. In the
> +	 * 2nd time, the user modes are filled, one by one from the modelist.
>  	 */
>  	if ((out_resp->count_modes >= mode_count) && mode_count) {
>  		copied = 0;
>  		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
> -		list_for_each_entry(mode, &connector->modes, head) {
> -			if (!drm_mode_expose_to_userspace(mode, file_priv))
> -				continue;
> -
> +		list_for_each_entry(mode, &modelist.head, head) {
>  			drm_mode_convert_to_umode(&u_mode, mode);
> +			/*
> +			 * Reset aspect ratio flags of user-mode, if modes with
> +			 * aspect-ratio are not supported.
> +			 */
> +			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
>  			if (copy_to_user(mode_ptr + copied,
>  					 &u_mode, sizeof(u_mode))) {
>  				ret = -EFAULT;
> @@ -1651,6 +1685,12 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>  
>  out:
> +	list_for_each_entry_safe(mode, tmp, &modelist.head, head) {
> +		list_del(&mode->head);
> +		drm_mode_destroy(dev, mode);
> +	}
> +	list_del(&modelist.head);
> +
>  	drm_connector_put(connector);
>  
>  	return ret;
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer
URL   : https://patchwork.freedesktop.org/series/42030/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
004a2f4d2e45 drm/modes: Introduce drm_mode_match()
-:39: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#39: FILE: drivers/gpu/drm/drm_modes.c:958:
+static bool drm_mode_match_clock(const struct drm_display_mode *mode1,
+				  const struct drm_display_mode *mode2)

total: 0 errors, 0 warnings, 1 checks, 190 lines checked
0b0dac34cf2c drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
dcd05d711f36 drm/edid: Fix cea mode aspect ratio handling
b381ceee1f16 drm/edid: Don't send bogus aspect ratios in AVI infoframes
e50db7ee7284 video/hdmi: Reject illegal picture aspect ratios
dd8bafc9bdc4 drm: Add DRM client cap for aspect-ratio
2005b3541205 drm: Add helper functions to handle aspect-ratio flag bits
f0bda1152c07 drm: Handle aspect ratio info in legacy and atomic modeset paths
-:77: CHECK:SPACING: No space is necessary after a cast
#77: FILE: drivers/gpu/drm/drm_atomic.c:399:
+		u_mode = (struct drm_mode_modeinfo *) blob->data;

total: 0 errors, 0 warnings, 1 checks, 185 lines checked
91a9c4be409c drm: Expose modes with aspect ratio, only if requested
bd2199516276 drm: Add aspect ratio parsing in DRM layer
-:86: CHECK:SPACING: space preferred before that '|' (ctx:VxE)
#86: FILE: drivers/gpu/drm/drm_modes.c:1052:
+			      DRM_MODE_MATCH_3D_FLAGS|
 			                             ^

total: 0 errors, 0 warnings, 1 checks, 77 lines checked
7908fbad00de drm: Add and handle new aspect ratios in DRM layer
-:89: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#89: FILE: include/uapi/drm/drm_mode.h:108:
+			(DRM_MODE_PICTURE_ASPECT_64_27<<19)
 			                              ^

-:91: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#91: FILE: include/uapi/drm/drm_mode.h:110:
+			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
 			                                ^

total: 0 errors, 0 warnings, 2 checks, 42 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Aspect ratio support in DRM layer

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer
URL   : https://patchwork.freedesktop.org/series/42030/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4072 -> Patchwork_8766 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42030/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_8766 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@kms_chamelium@hdmi-hpd-fast:
      fi-kbl-7500u:       SKIP -> FAIL (fdo#102672, fdo#103841)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084


== Participating hosts (33 -> 32) ==

  Additional (1): fi-kbl-7560u 
  Missing    (2): fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4072 -> Patchwork_8766

  CI_DRM_4072: b35e59e5c6a9cae11d5183d2bf9c5c99ceedbc7c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4442: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8766: 7908fbad00dea37dea9c50bd54356fd477db9fc6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4442: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

7908fbad00de drm: Add and handle new aspect ratios in DRM layer
bd2199516276 drm: Add aspect ratio parsing in DRM layer
91a9c4be409c drm: Expose modes with aspect ratio, only if requested
f0bda1152c07 drm: Handle aspect ratio info in legacy and atomic modeset paths
2005b3541205 drm: Add helper functions to handle aspect-ratio flag bits
dd8bafc9bdc4 drm: Add DRM client cap for aspect-ratio
e50db7ee7284 video/hdmi: Reject illegal picture aspect ratios
b381ceee1f16 drm/edid: Don't send bogus aspect ratios in AVI infoframes
dcd05d711f36 drm/edid: Fix cea mode aspect ratio handling
0b0dac34cf2c drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
004a2f4d2e45 drm/modes: Introduce drm_mode_match()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8766/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for Aspect ratio support in DRM layer

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer
URL   : https://patchwork.freedesktop.org/series/42030/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4072_full -> Patchwork_8766_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_8766_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8766_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42030/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8766_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_eio@reset-stress:
      shard-kbl:          PASS -> FAIL

    igt@gem_exec_suspend@basic-s3-devices:
      shard-hsw:          PASS -> DMESG-WARN

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          SKIP -> PASS +1

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c-frame-sequence:
      shard-glk:          PASS -> SKIP +96

    igt@kms_vblank@pipe-b-wait-forked-busy-hang:
      shard-glk:          SKIP -> PASS +120

    
== Known issues ==

  Here are the changes found in Patchwork_8766_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#105707)

    igt@kms_flip@absolute-wf_vblank-interruptible:
      shard-glk:          SKIP -> FAIL (fdo#106087)

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368) +1

    igt@kms_flip@plain-flip-ts-check-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103928)

    
    ==== Possible fixes ====

    igt@kms_flip@dpms-vs-vblank-race-interruptible:
      shard-glk:          FAIL (fdo#103060) -> PASS

    igt@kms_flip@plain-flip-fb-recreate:
      shard-hsw:          FAIL (fdo#100368) -> PASS
      shard-glk:          FAIL (fdo#100368) -> SKIP

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    igt@perf@blocking:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707
  fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4072 -> Patchwork_8766

  CI_DRM_4072: b35e59e5c6a9cae11d5183d2bf9c5c99ceedbc7c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4442: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8766: 7908fbad00dea37dea9c50bd54356fd477db9fc6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4442: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8766/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v11 06/11] drm: Add DRM client cap for aspect-ratio

[Nautiyal, Ankit K]

[-- Attachment #1.1: Type: text/plain, Size: 4054 bytes --]



On 4/20/2018 7:37 PM, Ville Syrjälä wrote:
> On Fri, Apr 20, 2018 at 07:01:46PM +0530, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> To enable aspect-ratio support in DRM, blindly exposing the aspect
>> ratio information along with mode, can break things in existing
>> user-spaces which have no intention or support to use this aspect
>> ratio information.
>>
>> To avoid this, a new drm client cap is required to enable a
>> user-space to advertise if it supports modes with aspect-ratio. Based
>> on this cap value, the kernel will take a call on exposing the aspect
>> ratio info in modes or not.
>>
>> This patch adds the client cap for aspect-ratio.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> V3: rebase
>> V4: As suggested by Marteen Lankhorst modified the commit message
>>      explaining the need to use the DRM cap for aspect-ratio. Also,
>>      tweaked the comment lines in the code for better understanding and
>>      clarity, as recommended by Shashank Sharma.
>> V5: rebase
>> V6: rebase
>> V7: rebase
>> V8: rebase
>> V9: rebase
>> V10: added comment explaining that no userspace breaks on aspect-ratio
>>       mode bits.
>>
>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/drm_ioctl.c | 9 +++++++++
>>   include/drm/drm_file.h      | 8 ++++++++
>>   include/uapi/drm/drm.h      | 7 +++++++
>>   3 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index af78291..39c8eab 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -325,6 +325,15 @@ 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;
>> +	/*
>> +	 * No Atomic userspace blows up on aspect ratio mode bits. Checked in
>> +	 * wayland/weston, xserver, and hardware-composer modeset paths.
>> +	 */
> Bogus indentation.

Thanks to point that out, will fix this.

> Also where's the aspect_ratio_allowed handling for the atomic cap?
> Or did we decide against it after all?

As discussed, aspect ratio is handled in the atomic modeset path, where 
in the modeset requests with aspect-ratios
are rejected, if the aspect-ratio cap not set.
The part which is dropped is - hiding the aspect-ratio information, 
while returning a mode blob in the drm_mode_get_blob
if the cap is not set. The patch was dropped as no user-space currently 
uses getblob ioctl to get the mode-blob.

Regards,
Ankit
>
>> +		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;
>> -- 
>> 2.7.4


[-- Attachment #1.2: Type: text/html, Size: 5167 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v11 07/11] drm: Add helper functions to handle aspect-ratio flag bits

[Nautiyal, Ankit K]

[-- Attachment #1.1: Type: text/plain, Size: 4209 bytes --]



On 4/20/2018 7:42 PM, Ville Syrjälä wrote:
> On Fri, Apr 20, 2018 at 07:01:47PM +0530, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> This patch adds helper functions for determining if aspect-ratio is
>> expected in user-mode and for allowing/disallowing the aspect-ratio,
>> if its not expected.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/drm_modes.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_modes.h     |  4 ++++
>>   2 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index c395a24..d6133e8 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -1759,3 +1759,50 @@ bool drm_mode_is_420(const struct drm_display_info *display,
>>   		drm_mode_is_420_also(display, mode);
>>   }
>>   EXPORT_SYMBOL(drm_mode_is_420);
>> +
>> +/**
>> + * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information
>> + * is expected from the user-mode.
>> + *
>> + * If the user has set aspect-ratio cap, then the flag of the user-mode is
>> + * allowed to contain aspect-ratio value.
>> + * If the user does not set aspect-ratio cap, then the only value allowed in the
>> + * flags bits is aspect-ratio NONE.
>> + *
>> + * @file_priv: file private structure to get the user capabilities
>> + * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio
>> + * information.
>> + *
>> + * Returns:
>> + * true if the aspect-ratio info is allowed in the user-mode flags.
>> + * false, otherwise.
>> + */
>> +bool
>> +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
>> +			      struct drm_mode_modeinfo *umode)
>> +{
>> +	return file_priv->aspect_ratio_allowed || (umode->flags &
>> +		DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;
> Odd line split here. Makes this a bit hard to read.
> I would split after the ||

Agreed. I wasn't sure how to let it have better readability and less 
than 80 char length
at the same time. Will fix this.

>
>> +}
>> +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed);
> Do we actually need to export these? I don't think so.
>
> But I might be wrong. It's a bit hard to see with the way
> you split this patch with the actual users in a different patch.

These helper functions are used in drm_mode_atomic.c, drm_mode_crtc.c, 
and drm_mode_get_connector.c
The patches are split as to have modes-set handling separate and the 
exposing of connector separate.
Do you suggest this patch to be merged with the patch for handling 
aspect-ratio in modeset?

>> +
>> +/**
>> + * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the
>> + * user-mode flags.
>> + *
>> + * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio
>> + * bits in the user-mode flags, if aspect-ratio info is not allowed.
>> + *
>> + * @file_priv: file private structure to get the user capabilities.
>> + * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to
>> + * be filtered.
>> + *
>> + */
>> +void
>> +drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
>> +				   struct drm_mode_modeinfo *umode)
>> +{
>> +	if (!drm_mode_aspect_ratio_allowed(file_priv, umode))
>> +		umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
>> +}
>> +EXPORT_SYMBOL(drm_mode_filter_aspect_ratio_flags);
>> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
>> index 2f78b7e..e0b060d 100644
>> --- a/include/drm/drm_modes.h
>> +++ b/include/drm/drm_modes.h
>> @@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display,
>>   			  const struct drm_display_mode *mode);
>>   bool drm_mode_is_420(const struct drm_display_info *display,
>>   		     const struct drm_display_mode *mode);
>> +bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
>> +				   struct drm_mode_modeinfo *umode);
>> +void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
>> +					struct drm_mode_modeinfo *umode);
>>   
>>   struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
>>   				      int hdisplay, int vdisplay, int vrefresh,
>> -- 
>> 2.7.4


[-- Attachment #1.2: Type: text/html, Size: 5201 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v11 09/11] drm: Expose modes with aspect ratio, only if requested

[Nautiyal, Ankit K]

[-- Attachment #1.1: Type: text/plain, Size: 7131 bytes --]



On 4/20/2018 7:52 PM, Ville Syrjälä wrote:
> On Fri, Apr 20, 2018 at 07:01:49PM +0530, Nautiyal, Ankit K wrote:
>> 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, regardless 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. However if such a mode is unique in the list, it is
>> kept in the list, with aspect-ratio flags reset.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> V3: As suggested by Ville, modified the mechanism of pruning of modes
>>      with aspect-ratio, if the aspect-ratio is not supported. Instead
>>      of straight away pruning such a mode, the mode is retained with
>>      aspect ratio bits set to zero, provided it is unique.
>> V4: rebase
>> V5: Addressed review comments from Ville:
>>      -used a pointer to store last valid mode.
>>      -avoided, modifying of picture_aspect_ratio in kernel mode,
>>       instead only flags bits of user mode are reset (if aspect-ratio
>>       is not supported).
>> V6: As suggested by Ville, corrected the mode pruning logic and
>>      elaborated the mode pruning logic and the assumptions taken.
>> V7: rebase
>> V8: rebase
>> V9: rebase
>> V10: rebase
>> V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
>>       logic to correctly identify and prune modes with aspect-ratio,
>>       if aspect-ratio cap is not set.
>> ---
>>   drivers/gpu/drm/drm_connector.c | 56 +++++++++++++++++++++++++++++++++++------
>>   1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index b3cde89..865ee354 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1531,15 +1531,35 @@ 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 *modelist,
>> +			     const struct drm_file *file_priv)
>>   {
>>   	/*
>>   	 * If user-space hasn't configured the driver to expose the stereo 3D
>>   	 * modes, don't expose them.
>>   	 */
>> +	struct drm_display_mode *mode_itr;
>> +
>>   	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. However if such a mode
>> +	 * is unique, let it be exposed, but reset the aspect-ratio flags
>> +	 * while preparing the list of user-modes.
>> +	 */
>> +	if (!file_priv->aspect_ratio_allowed &&
>> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
>> +		list_for_each_entry(mode_itr, &modelist->head, head)
>> +			if (drm_mode_match(mode_itr, mode,
>> +					   DRM_MODE_MATCH_TIMINGS |
>> +					   DRM_MODE_MATCH_CLOCK |
>> +					   DRM_MODE_MATCH_FLAGS |
>> +					   DRM_MODE_MATCH_3D_FLAGS))
>> +				return false;
>> +	}
>>   
>>   	return true;
>>   }
>> @@ -1550,7 +1570,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>>   	struct drm_mode_get_connector *out_resp = data;
>>   	struct drm_connector *connector;
>>   	struct drm_encoder *encoder;
>> -	struct drm_display_mode *mode;
>> +	struct drm_display_mode *mode, *tmp, modelist;
>>   	int mode_count = 0;
>>   	int encoders_count = 0;
>>   	int ret = 0;
>> @@ -1605,23 +1625,37 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>>   	out_resp->subpixel = connector->display_info.subpixel_order;
>>   	out_resp->connection = connector->status;
>>   
>> +	INIT_LIST_HEAD(&modelist.head);
> Why are we using a struct drm_display_mode to get a simple list_head?

Yes you are right, we can use the simple list_head here. I was trying to 
use modelist as first mode, and goofed up.
Its a mistake, I will correct it in next patchset.

>
>> +
>>   	/* delayed so we get modes regardless of pre-fill_modes state */
>>   	list_for_each_entry(mode, &connector->modes, head)
>> -		if (drm_mode_expose_to_userspace(mode, file_priv))
>> +		if (drm_mode_expose_to_userspace(mode, &modelist,
>> +						 file_priv)) {
>> +			struct drm_display_mode *tmp_mode;
>> +
>> +			tmp_mode = drm_mode_duplicate(dev, mode);
> Duplicating every mode seems rather wasteful. I suppose we could
> just add another list_head to struct drm_display_mode for this
> purpose. An alternative could be to somehow mark each exposed mode
> and just walk the original connector mode list.

Agreed. Its indeed wasteful as we have to make the list in 2nd ioctl 
call again.
As you have suggested, to mark the exposed modes, can we add a flag in 
drm_display_mode
something like "to_be_exposed", which will be set in the first ioctl 
call while determining the space.
2nd time, we can skip the marking process, and iterate over the 
connector mode-list, making
user-modes for only the modes that are marked?

>> +			list_add_tail(&tmp_mode->head, &modelist.head);
>>   			mode_count++;
>> +		}
>>   
>>   	/*
>>   	 * This ioctl is called twice, once to determine how much space is
>>   	 * needed, and the 2nd time to fill it.
>> +	 * The modes that need to be exposed to the user are maintained in the
>> +	 * 'modelist'. When the ioctl is called first time to determine the,
>> +	 * space, the modelist gets filled, to find the no. of modes. In the
>> +	 * 2nd time, the user modes are filled, one by one from the modelist.
>>   	 */
>>   	if ((out_resp->count_modes >= mode_count) && mode_count) {
>>   		copied = 0;
>>   		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
>> -		list_for_each_entry(mode, &connector->modes, head) {
>> -			if (!drm_mode_expose_to_userspace(mode, file_priv))
>> -				continue;
>> -
>> +		list_for_each_entry(mode, &modelist.head, head) {
>>   			drm_mode_convert_to_umode(&u_mode, mode);
>> +			/*
>> +			 * Reset aspect ratio flags of user-mode, if modes with
>> +			 * aspect-ratio are not supported.
>> +			 */
>> +			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
>>   			if (copy_to_user(mode_ptr + copied,
>>   					 &u_mode, sizeof(u_mode))) {
>>   				ret = -EFAULT;
>> @@ -1651,6 +1685,12 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>>   	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>   
>>   out:
>> +	list_for_each_entry_safe(mode, tmp, &modelist.head, head) {
>> +		list_del(&mode->head);
>> +		drm_mode_destroy(dev, mode);
>> +	}
>> +	list_del(&modelist.head);
>> +
>>   	drm_connector_put(connector);
>>   
>>   	return ret;
>> -- 
>> 2.7.4


[-- Attachment #1.2: Type: text/html, Size: 8246 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v11 06/11] drm: Add DRM client cap for aspect-ratio

[Ville Syrjälä]

On Mon, Apr 23, 2018 at 10:43:47AM +0530, Nautiyal, Ankit K wrote:
> 
> 
> On 4/20/2018 7:37 PM, Ville Syrjälä wrote:
> > On Fri, Apr 20, 2018 at 07:01:46PM +0530, Nautiyal, Ankit K wrote:
> >> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >>
> >> To enable aspect-ratio support in DRM, blindly exposing the aspect
> >> ratio information along with mode, can break things in existing
> >> user-spaces which have no intention or support to use this aspect
> >> ratio information.
> >>
> >> To avoid this, a new drm client cap is required to enable a
> >> user-space to advertise if it supports modes with aspect-ratio. Based
> >> on this cap value, the kernel will take a call on exposing the aspect
> >> ratio info in modes or not.
> >>
> >> This patch adds the client cap for aspect-ratio.
> >>
> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Cc: Shashank Sharma <shashank.sharma@intel.com>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >>
> >> V3: rebase
> >> V4: As suggested by Marteen Lankhorst modified the commit message
> >>      explaining the need to use the DRM cap for aspect-ratio. Also,
> >>      tweaked the comment lines in the code for better understanding and
> >>      clarity, as recommended by Shashank Sharma.
> >> V5: rebase
> >> V6: rebase
> >> V7: rebase
> >> V8: rebase
> >> V9: rebase
> >> V10: added comment explaining that no userspace breaks on aspect-ratio
> >>       mode bits.
> >>
> >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/drm_ioctl.c | 9 +++++++++
> >>   include/drm/drm_file.h      | 8 ++++++++
> >>   include/uapi/drm/drm.h      | 7 +++++++
> >>   3 files changed, 24 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> >> index af78291..39c8eab 100644
> >> --- a/drivers/gpu/drm/drm_ioctl.c
> >> +++ b/drivers/gpu/drm/drm_ioctl.c
> >> @@ -325,6 +325,15 @@ 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;
> >> +	/*
> >> +	 * No Atomic userspace blows up on aspect ratio mode bits. Checked in
> >> +	 * wayland/weston, xserver, and hardware-composer modeset paths.
> >> +	 */
> > Bogus indentation.
> 
> Thanks to point that out, will fix this.
> 
> > Also where's the aspect_ratio_allowed handling for the atomic cap?
> > Or did we decide against it after all?
> 
> As discussed, aspect ratio is handled in the atomic modeset path, where 
> in the modeset requests with aspect-ratios
> are rejected, if the aspect-ratio cap not set.

That is not what we discussed on irc. What Daniel was suggesting is
always enabling the aspect ratio cap for atomic clients, just as we
already enable the univerals planes cap for atomic clients.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v11 07/11] drm: Add helper functions to handle aspect-ratio flag bits

[Ville Syrjälä]

On Mon, Apr 23, 2018 at 10:55:54AM +0530, Nautiyal, Ankit K wrote:
> 
> 
> On 4/20/2018 7:42 PM, Ville Syrjälä wrote:
> > On Fri, Apr 20, 2018 at 07:01:47PM +0530, Nautiyal, Ankit K wrote:
> >> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >>
> >> This patch adds helper functions for determining if aspect-ratio is
> >> expected in user-mode and for allowing/disallowing the aspect-ratio,
> >> if its not expected.
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> ---
> >>   drivers/gpu/drm/drm_modes.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
> >>   include/drm/drm_modes.h     |  4 ++++
> >>   2 files changed, 51 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> >> index c395a24..d6133e8 100644
> >> --- a/drivers/gpu/drm/drm_modes.c
> >> +++ b/drivers/gpu/drm/drm_modes.c
> >> @@ -1759,3 +1759,50 @@ bool drm_mode_is_420(const struct drm_display_info *display,
> >>   		drm_mode_is_420_also(display, mode);
> >>   }
> >>   EXPORT_SYMBOL(drm_mode_is_420);
> >> +
> >> +/**
> >> + * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information
> >> + * is expected from the user-mode.
> >> + *
> >> + * If the user has set aspect-ratio cap, then the flag of the user-mode is
> >> + * allowed to contain aspect-ratio value.
> >> + * If the user does not set aspect-ratio cap, then the only value allowed in the
> >> + * flags bits is aspect-ratio NONE.
> >> + *
> >> + * @file_priv: file private structure to get the user capabilities
> >> + * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio
> >> + * information.
> >> + *
> >> + * Returns:
> >> + * true if the aspect-ratio info is allowed in the user-mode flags.
> >> + * false, otherwise.
> >> + */
> >> +bool
> >> +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
> >> +			      struct drm_mode_modeinfo *umode)
> >> +{
> >> +	return file_priv->aspect_ratio_allowed || (umode->flags &
> >> +		DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;
> > Odd line split here. Makes this a bit hard to read.
> > I would split after the ||
> 
> Agreed. I wasn't sure how to let it have better readability and less 
> than 80 char length
> at the same time. Will fix this.
> 
> >
> >> +}
> >> +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed);
> > Do we actually need to export these? I don't think so.
> >
> > But I might be wrong. It's a bit hard to see with the way
> > you split this patch with the actual users in a different patch.
> 
> These helper functions are used in drm_mode_atomic.c, drm_mode_crtc.c, 
> and drm_mode_get_connector.c
> The patches are split as to have modes-set handling separate and the 
> exposing of connector separate.
> Do you suggest this patch to be merged with the patch for handling 
> aspect-ratio in modeset?

Usually if you add a function you should do it in the same patch where
you add the user(s) for said function. Otherwise it's impossibly to
judge whether the new function is any good.

> 
> >> +
> >> +/**
> >> + * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the
> >> + * user-mode flags.
> >> + *
> >> + * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio
> >> + * bits in the user-mode flags, if aspect-ratio info is not allowed.
> >> + *
> >> + * @file_priv: file private structure to get the user capabilities.
> >> + * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to
> >> + * be filtered.
> >> + *
> >> + */
> >> +void
> >> +drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
> >> +				   struct drm_mode_modeinfo *umode)
> >> +{
> >> +	if (!drm_mode_aspect_ratio_allowed(file_priv, umode))
> >> +		umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
> >> +}
> >> +EXPORT_SYMBOL(drm_mode_filter_aspect_ratio_flags);
> >> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> >> index 2f78b7e..e0b060d 100644
> >> --- a/include/drm/drm_modes.h
> >> +++ b/include/drm/drm_modes.h
> >> @@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display,
> >>   			  const struct drm_display_mode *mode);
> >>   bool drm_mode_is_420(const struct drm_display_info *display,
> >>   		     const struct drm_display_mode *mode);
> >> +bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
> >> +				   struct drm_mode_modeinfo *umode);
> >> +void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
> >> +					struct drm_mode_modeinfo *umode);
> >>   
> >>   struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
> >>   				      int hdisplay, int vdisplay, int vrefresh,
> >> -- 
> >> 2.7.4
> 

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v11 09/11] drm: Expose modes with aspect ratio, only if requested

[Ville Syrjälä]

On Mon, Apr 23, 2018 at 11:19:08AM +0530, Nautiyal, Ankit K wrote:
> 
> 
> On 4/20/2018 7:52 PM, Ville Syrjälä wrote:
> > On Fri, Apr 20, 2018 at 07:01:49PM +0530, Nautiyal, Ankit K wrote:
> >> 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, regardless 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. However if such a mode is unique in the list, it is
> >> kept in the list, with aspect-ratio flags reset.
> >>
> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Cc: Shashank Sharma <shashank.sharma@intel.com>
> >> Cc: Jose Abreu <jose.abreu@synopsys.com>
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >>
> >> V3: As suggested by Ville, modified the mechanism of pruning of modes
> >>      with aspect-ratio, if the aspect-ratio is not supported. Instead
> >>      of straight away pruning such a mode, the mode is retained with
> >>      aspect ratio bits set to zero, provided it is unique.
> >> V4: rebase
> >> V5: Addressed review comments from Ville:
> >>      -used a pointer to store last valid mode.
> >>      -avoided, modifying of picture_aspect_ratio in kernel mode,
> >>       instead only flags bits of user mode are reset (if aspect-ratio
> >>       is not supported).
> >> V6: As suggested by Ville, corrected the mode pruning logic and
> >>      elaborated the mode pruning logic and the assumptions taken.
> >> V7: rebase
> >> V8: rebase
> >> V9: rebase
> >> V10: rebase
> >> V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
> >>       logic to correctly identify and prune modes with aspect-ratio,
> >>       if aspect-ratio cap is not set.
> >> ---
> >>   drivers/gpu/drm/drm_connector.c | 56 +++++++++++++++++++++++++++++++++++------
> >>   1 file changed, 48 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> >> index b3cde89..865ee354 100644
> >> --- a/drivers/gpu/drm/drm_connector.c
> >> +++ b/drivers/gpu/drm/drm_connector.c
> >> @@ -1531,15 +1531,35 @@ 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 *modelist,
> >> +			     const struct drm_file *file_priv)
> >>   {
> >>   	/*
> >>   	 * If user-space hasn't configured the driver to expose the stereo 3D
> >>   	 * modes, don't expose them.
> >>   	 */
> >> +	struct drm_display_mode *mode_itr;
> >> +
> >>   	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. However if such a mode
> >> +	 * is unique, let it be exposed, but reset the aspect-ratio flags
> >> +	 * while preparing the list of user-modes.
> >> +	 */
> >> +	if (!file_priv->aspect_ratio_allowed &&
> >> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
> >> +		list_for_each_entry(mode_itr, &modelist->head, head)
> >> +			if (drm_mode_match(mode_itr, mode,
> >> +					   DRM_MODE_MATCH_TIMINGS |
> >> +					   DRM_MODE_MATCH_CLOCK |
> >> +					   DRM_MODE_MATCH_FLAGS |
> >> +					   DRM_MODE_MATCH_3D_FLAGS))
> >> +				return false;
> >> +	}
> >>   
> >>   	return true;
> >>   }
> >> @@ -1550,7 +1570,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> >>   	struct drm_mode_get_connector *out_resp = data;
> >>   	struct drm_connector *connector;
> >>   	struct drm_encoder *encoder;
> >> -	struct drm_display_mode *mode;
> >> +	struct drm_display_mode *mode, *tmp, modelist;
> >>   	int mode_count = 0;
> >>   	int encoders_count = 0;
> >>   	int ret = 0;
> >> @@ -1605,23 +1625,37 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> >>   	out_resp->subpixel = connector->display_info.subpixel_order;
> >>   	out_resp->connection = connector->status;
> >>   
> >> +	INIT_LIST_HEAD(&modelist.head);
> > Why are we using a struct drm_display_mode to get a simple list_head?
> 
> Yes you are right, we can use the simple list_head here. I was trying to 
> use modelist as first mode, and goofed up.
> Its a mistake, I will correct it in next patchset.
> 
> >
> >> +
> >>   	/* delayed so we get modes regardless of pre-fill_modes state */
> >>   	list_for_each_entry(mode, &connector->modes, head)
> >> -		if (drm_mode_expose_to_userspace(mode, file_priv))
> >> +		if (drm_mode_expose_to_userspace(mode, &modelist,
> >> +						 file_priv)) {
> >> +			struct drm_display_mode *tmp_mode;
> >> +
> >> +			tmp_mode = drm_mode_duplicate(dev, mode);
> > Duplicating every mode seems rather wasteful. I suppose we could
> > just add another list_head to struct drm_display_mode for this
> > purpose. An alternative could be to somehow mark each exposed mode
> > and just walk the original connector mode list.
> 
> Agreed. Its indeed wasteful as we have to make the list in 2nd ioctl 
> call again.
> As you have suggested, to mark the exposed modes, can we add a flag in 
> drm_display_mode
> something like "to_be_exposed", which will be set in the first ioctl 
> call while determining the space.
> 2nd time, we can skip the marking process, and iterate over the 
> connector mode-list, making
> user-modes for only the modes that are marked?

No. You can't trust such a flag across two ioctl calls. It would have to
be protected by mode_config.mutex and as soon as you'd drop the lock
the flag would become invalid.

> 
> >> +			list_add_tail(&tmp_mode->head, &modelist.head);
> >>   			mode_count++;
> >> +		}
> >>   
> >>   	/*
> >>   	 * This ioctl is called twice, once to determine how much space is
> >>   	 * needed, and the 2nd time to fill it.
> >> +	 * The modes that need to be exposed to the user are maintained in the
> >> +	 * 'modelist'. When the ioctl is called first time to determine the,
> >> +	 * space, the modelist gets filled, to find the no. of modes. In the
> >> +	 * 2nd time, the user modes are filled, one by one from the modelist.
> >>   	 */
> >>   	if ((out_resp->count_modes >= mode_count) && mode_count) {
> >>   		copied = 0;
> >>   		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
> >> -		list_for_each_entry(mode, &connector->modes, head) {
> >> -			if (!drm_mode_expose_to_userspace(mode, file_priv))
> >> -				continue;
> >> -
> >> +		list_for_each_entry(mode, &modelist.head, head) {
> >>   			drm_mode_convert_to_umode(&u_mode, mode);
> >> +			/*
> >> +			 * Reset aspect ratio flags of user-mode, if modes with
> >> +			 * aspect-ratio are not supported.
> >> +			 */
> >> +			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
> >>   			if (copy_to_user(mode_ptr + copied,
> >>   					 &u_mode, sizeof(u_mode))) {
> >>   				ret = -EFAULT;
> >> @@ -1651,6 +1685,12 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> >>   	drm_modeset_unlock(&dev->mode_config.connection_mutex);
> >>   
> >>   out:
> >> +	list_for_each_entry_safe(mode, tmp, &modelist.head, head) {
> >> +		list_del(&mode->head);
> >> +		drm_mode_destroy(dev, mode);
> >> +	}
> >> +	list_del(&modelist.head);
> >> +
> >>   	drm_connector_put(connector);
> >>   
> >>   	return ret;
> >> -- 
> >> 2.7.4
> 

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v11 07/11] drm: Add helper functions to handle aspect-ratio flag bits

[Jani Nikula]

On Mon, 23 Apr 2018, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
> On 4/20/2018 7:42 PM, Ville Syrjälä wrote:
>> On Fri, Apr 20, 2018 at 07:01:47PM +0530, Nautiyal, Ankit K wrote:
>>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>> +bool
>>> +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
>>> +			      struct drm_mode_modeinfo *umode)
>>> +{
>>> +	return file_priv->aspect_ratio_allowed || (umode->flags &
>>> +		DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;
>> Odd line split here. Makes this a bit hard to read.
>> I would split after the ||
>
> Agreed. I wasn't sure how to let it have better readability and less
> than 80 char length at the same time. Will fix this.

80 chars is not a strict requirement. Readability is. ;)

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v11 07/11] drm: Add helper functions to handle aspect-ratio flag bits

[Nautiyal, Ankit K]

On 4/23/2018 3:52 PM, Jani Nikula wrote:
> On Mon, 23 Apr 2018, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
>> On 4/20/2018 7:42 PM, Ville Syrjälä wrote:
>>> On Fri, Apr 20, 2018 at 07:01:47PM +0530, Nautiyal, Ankit K wrote:
>>>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>> +bool
>>>> +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
>>>> +			      struct drm_mode_modeinfo *umode)
>>>> +{
>>>> +	return file_priv->aspect_ratio_allowed || (umode->flags &
>>>> +		DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;
>>> Odd line split here. Makes this a bit hard to read.
>>> I would split after the ||
>> Agreed. I wasn't sure how to let it have better readability and less
>> than 80 char length at the same time. Will fix this.
> 80 chars is not a strict requirement. Readability is. ;)
>
> BR,
> Jani.

Roger that! :)


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v11 07/11] drm: Add helper functions to handle aspect-ratio flag bits

[Nautiyal, Ankit K]

On 4/23/2018 3:43 PM, Ville Syrjälä wrote:
> On Mon, Apr 23, 2018 at 10:55:54AM +0530, Nautiyal, Ankit K wrote:
>>
>> On 4/20/2018 7:42 PM, Ville Syrjälä wrote:
>>> On Fri, Apr 20, 2018 at 07:01:47PM +0530, Nautiyal, Ankit K wrote:
>>>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>>
>>>> This patch adds helper functions for determining if aspect-ratio is
>>>> expected in user-mode and for allowing/disallowing the aspect-ratio,
>>>> if its not expected.
>>>>
>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/drm_modes.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
>>>>    include/drm/drm_modes.h     |  4 ++++
>>>>    2 files changed, 51 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>>>> index c395a24..d6133e8 100644
>>>> --- a/drivers/gpu/drm/drm_modes.c
>>>> +++ b/drivers/gpu/drm/drm_modes.c
>>>> @@ -1759,3 +1759,50 @@ bool drm_mode_is_420(const struct drm_display_info *display,
>>>>             drm_mode_is_420_also(display, mode);
>>>>    }
>>>>    EXPORT_SYMBOL(drm_mode_is_420);
>>>> +
>>>> +/**
>>>> + * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information
>>>> + * is expected from the user-mode.
>>>> + *
>>>> + * If the user has set aspect-ratio cap, then the flag of the user-mode is
>>>> + * allowed to contain aspect-ratio value.
>>>> + * If the user does not set aspect-ratio cap, then the only value allowed in the
>>>> + * flags bits is aspect-ratio NONE.
>>>> + *
>>>> + * @file_priv: file private structure to get the user capabilities
>>>> + * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio
>>>> + * information.
>>>> + *
>>>> + * Returns:
>>>> + * true if the aspect-ratio info is allowed in the user-mode flags.
>>>> + * false, otherwise.
>>>> + */
>>>> +bool
>>>> +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
>>>> +                        struct drm_mode_modeinfo *umode)
>>>> +{
>>>> +  return file_priv->aspect_ratio_allowed || (umode->flags &
>>>> +          DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE;
>>> Odd line split here. Makes this a bit hard to read.
>>> I would split after the ||
>> Agreed. I wasn't sure how to let it have better readability and less
>> than 80 char length
>> at the same time. Will fix this.
>>
>>>> +}
>>>> +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed);
>>> Do we actually need to export these? I don't think so.
>>>
>>> But I might be wrong. It's a bit hard to see with the way
>>> you split this patch with the actual users in a different patch.
>> These helper functions are used in drm_mode_atomic.c, drm_mode_crtc.c,
>> and drm_mode_get_connector.c
>> The patches are split as to have modes-set handling separate and the
>> exposing of connector separate.
>> Do you suggest this patch to be merged with the patch for handling
>> aspect-ratio in modeset?
> Usually if you add a function you should do it in the same patch where
> you add the user(s) for said function. Otherwise it's impossibly to
> judge whether the new function is any good.

That makes sense. I will merge this patch with the patch for handling 
aspect ratio in modeset path,
where its actually used.

>
>>>> +
>>>> +/**
>>>> + * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the
>>>> + * user-mode flags.
>>>> + *
>>>> + * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio
>>>> + * bits in the user-mode flags, if aspect-ratio info is not allowed.
>>>> + *
>>>> + * @file_priv: file private structure to get the user capabilities.
>>>> + * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to
>>>> + * be filtered.
>>>> + *
>>>> + */
>>>> +void
>>>> +drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
>>>> +                             struct drm_mode_modeinfo *umode)
>>>> +{
>>>> +  if (!drm_mode_aspect_ratio_allowed(file_priv, umode))
>>>> +          umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
>>>> +}
>>>> +EXPORT_SYMBOL(drm_mode_filter_aspect_ratio_flags);
>>>> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
>>>> index 2f78b7e..e0b060d 100644
>>>> --- a/include/drm/drm_modes.h
>>>> +++ b/include/drm/drm_modes.h
>>>> @@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display,
>>>>                       const struct drm_display_mode *mode);
>>>>    bool drm_mode_is_420(const struct drm_display_info *display,
>>>>                  const struct drm_display_mode *mode);
>>>> +bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv,
>>>> +                             struct drm_mode_modeinfo *umode);
>>>> +void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv,
>>>> +                                  struct drm_mode_modeinfo *umode);
>>>>
>>>>    struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
>>>>                                   int hdisplay, int vdisplay, int vrefresh,
>>>> --
>>>> 2.7.4
> --
> Ville Syrjälä
> Intel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH v11 06/11] drm: Add DRM client cap for aspect-ratio

[Ville Syrjälä]

On Mon, Apr 23, 2018 at 01:11:25PM +0300, Ville Syrjälä wrote:
> On Mon, Apr 23, 2018 at 10:43:47AM +0530, Nautiyal, Ankit K wrote:
> > 
> > 
> > On 4/20/2018 7:37 PM, Ville Syrjälä wrote:
> > > On Fri, Apr 20, 2018 at 07:01:46PM +0530, Nautiyal, Ankit K wrote:
> > >> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > >>
> > >> To enable aspect-ratio support in DRM, blindly exposing the aspect
> > >> ratio information along with mode, can break things in existing
> > >> user-spaces which have no intention or support to use this aspect
> > >> ratio information.
> > >>
> > >> To avoid this, a new drm client cap is required to enable a
> > >> user-space to advertise if it supports modes with aspect-ratio. Based
> > >> on this cap value, the kernel will take a call on exposing the aspect
> > >> ratio info in modes or not.
> > >>
> > >> This patch adds the client cap for aspect-ratio.
> > >>
> > >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > >> Cc: Shashank Sharma <shashank.sharma@intel.com>
> > >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > >>
> > >> V3: rebase
> > >> V4: As suggested by Marteen Lankhorst modified the commit message
> > >>      explaining the need to use the DRM cap for aspect-ratio. Also,
> > >>      tweaked the comment lines in the code for better understanding and
> > >>      clarity, as recommended by Shashank Sharma.
> > >> V5: rebase
> > >> V6: rebase
> > >> V7: rebase
> > >> V8: rebase
> > >> V9: rebase
> > >> V10: added comment explaining that no userspace breaks on aspect-ratio
> > >>       mode bits.
> > >>
> > >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> > >> ---
> > >>   drivers/gpu/drm/drm_ioctl.c | 9 +++++++++
> > >>   include/drm/drm_file.h      | 8 ++++++++
> > >>   include/uapi/drm/drm.h      | 7 +++++++
> > >>   3 files changed, 24 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > >> index af78291..39c8eab 100644
> > >> --- a/drivers/gpu/drm/drm_ioctl.c
> > >> +++ b/drivers/gpu/drm/drm_ioctl.c
> > >> @@ -325,6 +325,15 @@ 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;
> > >> +	/*
> > >> +	 * No Atomic userspace blows up on aspect ratio mode bits. Checked in
> > >> +	 * wayland/weston, xserver, and hardware-composer modeset paths.
> > >> +	 */
> > > Bogus indentation.
> > 
> > Thanks to point that out, will fix this.
> > 
> > > Also where's the aspect_ratio_allowed handling for the atomic cap?
> > > Or did we decide against it after all?
> > 
> > As discussed, aspect ratio is handled in the atomic modeset path, where 
> > in the modeset requests with aspect-ratios
> > are rejected, if the aspect-ratio cap not set.
> 
> That is not what we discussed on irc. What Daniel was suggesting is
> always enabling the aspect ratio cap for atomic clients, just as we
> already enable the univerals planes cap for atomic clients.

And to make sure we're on the same page finally

@@ -320,14 +320,15 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
        case DRM_CLIENT_CAP_ATOMIC:
                if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
                        return -EINVAL;
                if (req->value > 1)
                        return -EINVAL;
                file_priv->atomic = req->value;
                file_priv->universal_planes = req->value;
+               file_priv->aspect_ratio_allowed = req->value;
                break;
        default:
                return -EINVAL;
        }
 
        return 0;
 }

is what we're talking about here.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer
URL   : https://patchwork.freedesktop.org/series/42870/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
d895eaa49246 drm/modes: Introduce drm_mode_match()
-:39: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#39: FILE: drivers/gpu/drm/drm_modes.c:958:
+static bool drm_mode_match_clock(const struct drm_display_mode *mode1,
+				  const struct drm_display_mode *mode2)

total: 0 errors, 0 warnings, 1 checks, 190 lines checked
1b86affe6c5b drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
fbdba68c6683 drm/edid: Fix cea mode aspect ratio handling
78e0c2cb19c1 drm/edid: Don't send bogus aspect ratios in AVI infoframes
431a3a8e1cc3 video/hdmi: Reject illegal picture aspect ratios
239f348884ca drm: Add DRM client cap for aspect-ratio
6f1f8918f916 drm: Handle aspect ratio info in legacy modeset path
-:60: WARNING:LONG_LINE: line over 100 characters
#60: FILE: drivers/gpu/drm/drm_crtc.c:634:
+		    (crtc_req->mode.flags & DRM_MODE_FLAG_PIC_AR_MASK) != DRM_MODE_FLAG_PIC_AR_NONE) {

total: 0 errors, 1 warnings, 0 checks, 21 lines checked
90ee0eab33c1 drm: Expose modes with aspect ratio, only if requested
1f8eb37880e7 drm: Add aspect ratio parsing in DRM layer
-:90: CHECK:SPACING: space preferred before that '|' (ctx:VxE)
#90: FILE: drivers/gpu/drm/drm_modes.c:1052:
+			      DRM_MODE_MATCH_3D_FLAGS|
 			                             ^

total: 0 errors, 0 warnings, 1 checks, 77 lines checked
dbf66a2a1b32 drm: Add and handle new aspect ratios in DRM layer
-:93: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#93: FILE: include/uapi/drm/drm_mode.h:108:
+			(DRM_MODE_PICTURE_ASPECT_64_27<<19)
 			                              ^

-:95: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#95: FILE: include/uapi/drm/drm_mode.h:110:
+			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
 			                                ^

total: 0 errors, 0 warnings, 2 checks, 42 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer
URL   : https://patchwork.freedesktop.org/series/42546/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
10a7bea36941 drm/modes: Introduce drm_mode_match()
-:39: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#39: FILE: drivers/gpu/drm/drm_modes.c:958:
+static bool drm_mode_match_clock(const struct drm_display_mode *mode1,
+				  const struct drm_display_mode *mode2)

total: 0 errors, 0 warnings, 1 checks, 190 lines checked
2f25a53751d3 drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
7746cbe0b7a5 drm/edid: Fix cea mode aspect ratio handling
023012e2f0d0 drm/edid: Don't send bogus aspect ratios in AVI infoframes
67936f9d93c9 video/hdmi: Reject illegal picture aspect ratios
4c1ac075fbaf drm: Add DRM client cap for aspect-ratio
7188f4f6cd36 drm: Handle aspect ratio info in legacy modeset path
3e6df2a97a1a drm: Expose modes with aspect ratio, only if requested
e65cee05eba3 drm: Add aspect ratio parsing in DRM layer
-:89: CHECK:SPACING: space preferred before that '|' (ctx:VxE)
#89: FILE: drivers/gpu/drm/drm_modes.c:1052:
+			      DRM_MODE_MATCH_3D_FLAGS|
 			                             ^

total: 0 errors, 0 warnings, 1 checks, 77 lines checked
199615dff0c9 drm: Add and handle new aspect ratios in DRM layer
-:92: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#92: FILE: include/uapi/drm/drm_mode.h:108:
+			(DRM_MODE_PICTURE_ASPECT_64_27<<19)
 			                              ^

-:94: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#94: FILE: include/uapi/drm/drm_mode.h:110:
+			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
 			                                ^

total: 0 errors, 0 warnings, 2 checks, 42 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer
URL   : https://patchwork.freedesktop.org/series/42403/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
26bd319b4abf drm/modes: Introduce drm_mode_match()
-:39: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#39: FILE: drivers/gpu/drm/drm_modes.c:958:
+static bool drm_mode_match_clock(const struct drm_display_mode *mode1,
+				  const struct drm_display_mode *mode2)

total: 0 errors, 0 warnings, 1 checks, 190 lines checked
d13115ecbdc1 drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
a881ad03fda9 drm/edid: Fix cea mode aspect ratio handling
38dcb640b419 drm/edid: Don't send bogus aspect ratios in AVI infoframes
2daade285efb video/hdmi: Reject illegal picture aspect ratios
e86d5a0d9ee0 drm: Add DRM client cap for aspect-ratio
9955311be9aa drm: Handle aspect ratio info in legacy modeset path
2a72ce9d5a9a drm: Expose modes with aspect ratio, only if requested
1b7fe5d2c8aa drm: Add aspect ratio parsing in DRM layer
-:88: CHECK:SPACING: space preferred before that '|' (ctx:VxE)
#88: FILE: drivers/gpu/drm/drm_modes.c:1052:
+			      DRM_MODE_MATCH_3D_FLAGS|
 			                             ^

total: 0 errors, 0 warnings, 1 checks, 77 lines checked
8355947e8186 drm: Add and handle new aspect ratios in DRM layer
-:91: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#91: FILE: include/uapi/drm/drm_mode.h:108:
+			(DRM_MODE_PICTURE_ASPECT_64_27<<19)
 			                              ^

-:93: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#93: FILE: include/uapi/drm/drm_mode.h:110:
+			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
 			                                ^

total: 0 errors, 0 warnings, 2 checks, 42 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer
URL   : https://patchwork.freedesktop.org/series/42403/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
6d0e3d322715 drm/modes: Introduce drm_mode_match()
-:39: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#39: FILE: drivers/gpu/drm/drm_modes.c:958:
+static bool drm_mode_match_clock(const struct drm_display_mode *mode1,
+				  const struct drm_display_mode *mode2)

total: 0 errors, 0 warnings, 1 checks, 190 lines checked
f4fd4583689d drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
78e1b6d70ed4 drm/edid: Fix cea mode aspect ratio handling
d6886905af86 drm/edid: Don't send bogus aspect ratios in AVI infoframes
580bb243213b video/hdmi: Reject illegal picture aspect ratios
dc7e7e1e0257 drm: Add DRM client cap for aspect-ratio
f94f783b89d9 drm: Handle aspect ratio info in legacy modeset path
1ba4ec5d7d45 drm: Expose modes with aspect ratio, only if requested
58c770cef648 drm: Add aspect ratio parsing in DRM layer
-:88: CHECK:SPACING: space preferred before that '|' (ctx:VxE)
#88: FILE: drivers/gpu/drm/drm_modes.c:1052:
+			      DRM_MODE_MATCH_3D_FLAGS|
 			                             ^

total: 0 errors, 0 warnings, 1 checks, 77 lines checked
1cccf4dbbdca drm: Add and handle new aspect ratios in DRM layer
-:91: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#91: FILE: include/uapi/drm/drm_mode.h:108:
+			(DRM_MODE_PICTURE_ASPECT_64_27<<19)
 			                              ^

-:93: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#93: FILE: include/uapi/drm/drm_mode.h:110:
+			(DRM_MODE_PICTURE_ASPECT_256_135<<19)
 			                                ^

total: 0 errors, 0 warnings, 2 checks, 42 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx