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

[PATCH v10 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     |  40 ++++++-
 drivers/gpu/drm/drm_crtc.c          |   8 ++
 drivers/gpu/drm/drm_crtc_internal.h |   3 +-
 drivers/gpu/drm/drm_edid.c          |  41 +++++--
 drivers/gpu/drm/drm_fb_helper.c     |  12 +-
 drivers/gpu/drm/drm_ioctl.c         |   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, 365 insertions(+), 65 deletions(-)

-- 
2.7.4

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

[PATCH v10 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 v10 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

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

[PATCH v10 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

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

[PATCH v10 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

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

[PATCH v10 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 v10 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 v10 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

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

[PATCH v10 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 7d25c42..5863072 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) {
@@ -1961,7 +1976,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;
@@ -1995,7 +2011,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: {
@@ -2384,7 +2400,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 ee03c1e..b75343b 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;
 
@@ -2762,7 +2762,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;
 
@@ -2921,7 +2921,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 v10 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, regadless of
whether user space requested this information or not.

This patch prunes the modes with aspect-ratio information, from a
connector's modelist, if the user-space has not set the aspect ratio
DRM client cap.

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

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

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

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b3cde89..5420325 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1531,8 +1531,10 @@ static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne
 	return connector->encoder;
 }
 
-static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
-					 const struct drm_file *file_priv)
+static bool
+drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
+			     const struct drm_display_mode *last_mode,
+			     const struct drm_file *file_priv)
 {
 	/*
 	 * If user-space hasn't configured the driver to expose the stereo 3D
@@ -1540,6 +1542,26 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
 	 */
 	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
 		return false;
+	/*
+	 * If user-space hasn't configured the driver to expose the modes with
+	 * aspect-ratio, don't expose them. But in case of a unique mode, let
+	 * the mode be passed, so that it can be enumerated with aspect-ratio
+	 * bits erased.
+	 *
+	 * It is assumed here, that the list of modes for a given connector, is
+	 * sorted, such that modes that have different aspect-ratios, but are
+	 * otherwise identical, are back to back.
+	 * This way, saving the last valid mode, and matching it with the
+	 * current mode will help in determining, if the current mode is unique.
+	 */
+	if (!file_priv->aspect_ratio_allowed &&
+	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE &&
+	    last_mode && drm_mode_match(mode, last_mode,
+					DRM_MODE_MATCH_TIMINGS |
+					DRM_MODE_MATCH_CLOCK |
+					DRM_MODE_MATCH_FLAGS |
+					DRM_MODE_MATCH_3D_FLAGS))
+		return false;
 
 	return true;
 }
@@ -1551,6 +1573,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 	struct drm_display_mode *mode;
+	struct drm_display_mode *last_valid_mode;
 	int mode_count = 0;
 	int encoders_count = 0;
 	int ret = 0;
@@ -1606,9 +1629,13 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	out_resp->connection = connector->status;
 
 	/* delayed so we get modes regardless of pre-fill_modes state */
+	last_valid_mode = NULL;
 	list_for_each_entry(mode, &connector->modes, head)
-		if (drm_mode_expose_to_userspace(mode, file_priv))
+		if (drm_mode_expose_to_userspace(mode, last_valid_mode,
+						 file_priv)) {
 			mode_count++;
+			last_valid_mode = mode;
+		}
 
 	/*
 	 * This ioctl is called twice, once to determine how much space is
@@ -1617,11 +1644,15 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	if ((out_resp->count_modes >= mode_count) && mode_count) {
 		copied = 0;
 		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
+		last_valid_mode = NULL;
 		list_for_each_entry(mode, &connector->modes, head) {
-			if (!drm_mode_expose_to_userspace(mode, file_priv))
+			if (!drm_mode_expose_to_userspace(mode,
+							  last_valid_mode,
+							  file_priv))
 				continue;
 
 			drm_mode_convert_to_umode(&u_mode, mode);
+			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
 			if (copy_to_user(mode_ptr + copied,
 					 &u_mode, sizeof(u_mode))) {
 				ret = -EFAULT;
@@ -1629,6 +1660,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 
 				goto out;
 			}
+			last_valid_mode = mode;
 			copied++;
 		}
 	}
-- 
2.7.4

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

[PATCH v10 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

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

[PATCH v10 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

_______________________________________________
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 (rev2)

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer (rev2)
URL   : https://patchwork.freedesktop.org/series/39960/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
e9fb8279cbaa 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
c8c80aa31122 drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
7814e2b10e68 drm/edid: Fix cea mode aspect ratio handling
43c4528f5758 drm/edid: Don't send bogus aspect ratios in AVI infoframes
bb7278472467 video/hdmi: Reject illegal picture aspect ratios
73fd1b48e332 drm: Add DRM client cap for aspect-ratio
7044f8820325 drm: Add helper functions to handle aspect-ratio flag bits
03deca3a8769 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
3fcc37ccf3df drm: Expose modes with aspect ratio, only if requested
dc1b0391df73 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
41ab2195ed81 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

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

[Nautiyal, Ankit K]

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

This patch is causing failure of IGT test kms_3d. The kms_3d test 
expects the no. of 3d modes to be 13.

(The test has hard-coded value for expected no. of 3d modes as 13)

But due to the addition of "matching aspect_ratio" in drm_mode_equal in 
this patch, the total no. of

modes in the connector modelist is increased by 2, resulting in failure 
of assertion 'mode_count==13'.

Perhaps this need to be handled in the test.

-Regards,

Ankit


On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> 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;


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

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

_______________________________________________
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 (rev2)

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer (rev2)
URL   : https://patchwork.freedesktop.org/series/39960/
State : success

== Summary ==

Series 39960v2 Aspect ratio support in DRM layer
https://patchwork.freedesktop.org/api/1.0/series/39960/revisions/2/mbox/

---- Possible new issues:

Test drv_module_reload:
        Subgroup basic-no-display:
                incomplete -> PASS       (fi-elk-e7500)

---- Known issues:

Test kms_chamelium:
        Subgroup dp-crc-fast:
                pass       -> DMESG-FAIL (fi-kbl-7500u) fdo#105589
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-bxt-dsi) fdo#103927

fdo#105589 https://bugs.freedesktop.org/show_bug.cgi?id=105589
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:429s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:440s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:381s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:542s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:298s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:513s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:518s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:527s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:508s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:412s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:559s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:511s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:583s
fi-elk-e7500     total:285  pass:226  dwarn:0   dfail:0   fail:0   skip:59  time:424s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:313s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:545s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:491s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:404s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:422s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:473s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:432s
fi-kbl-7500u     total:285  pass:259  dwarn:1   dfail:1   fail:0   skip:24  time:474s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:463s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-pnv-d510      total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:657s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:441s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:538s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:504s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:501s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:433s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:443s
fi-snb-2520m     total:242  pass:208  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:407s

3cbdbfb1ecc8769c8db88e95b1d6ea55c5e87dbf drm-tip: 2018y-04m-06d-15h-15m-21s UTC integration manifest
41ab2195ed81 drm: Add and handle new aspect ratios in DRM layer
dc1b0391df73 drm: Add aspect ratio parsing in DRM layer
3fcc37ccf3df drm: Expose modes with aspect ratio, only if requested
03deca3a8769 drm: Handle aspect ratio info in legacy and atomic modeset paths
7044f8820325 drm: Add helper functions to handle aspect-ratio flag bits
73fd1b48e332 drm: Add DRM client cap for aspect-ratio
bb7278472467 video/hdmi: Reject illegal picture aspect ratios
43c4528f5758 drm/edid: Don't send bogus aspect ratios in AVI infoframes
7814e2b10e68 drm/edid: Fix cea mode aspect ratio handling
c8c80aa31122 drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
e9fb8279cbaa drm/modes: Introduce drm_mode_match()

== Logs ==

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

Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

[Ville Syrjälä]

On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
> This patch is causing failure of IGT test kms_3d. The kms_3d test 
> expects the no. of 3d modes to be 13.
> 
> (The test has hard-coded value for expected no. of 3d modes as 13)
> 
> But due to the addition of "matching aspect_ratio" in drm_mode_equal in 
> this patch, the total no. of
> 
> modes in the connector modelist is increased by 2, resulting in failure 
> of assertion 'mode_count==13'.

If kms_3d isn't setting the aspect ratio cap how is it affected by these
changes?

> 
> Perhaps this need to be handled in the test.
> 
> -Regards,
> 
> Ankit
> 
> 
> On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> > 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;
> 

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


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

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

[Patchwork]

== Series Details ==

Series: Aspect ratio support in DRM layer (rev2)
URL   : https://patchwork.freedesktop.org/series/39960/
State : failure

== Summary ==

---- Possible new issues:

Test gem_mmap_gtt:
        Subgroup forked-medium-copy-odd:
                dmesg-warn -> PASS       (shard-hsw)
Test kms_3d:
                pass       -> FAIL       (shard-snb)
Test kms_frontbuffer_tracking:
        Subgroup fbc-2p-pri-indfb-multidraw:
                dmesg-fail -> PASS       (shard-hsw)
Test pm_rc6_residency:
        Subgroup rc6-accuracy:
                skip       -> PASS       (shard-snb)

---- Known issues:

Test kms_cursor_legacy:
        Subgroup 2x-long-flip-vs-cursor-legacy:
                fail       -> PASS       (shard-hsw) fdo#104873
Test kms_flip:
        Subgroup dpms-vs-vblank-race:
                pass       -> FAIL       (shard-hsw) fdo#103060
        Subgroup plain-flip-fb-recreate:
                pass       -> FAIL       (shard-hsw) fdo#100368 +2
Test kms_rotation_crc:
        Subgroup primary-rotation-180:
                pass       -> FAIL       (shard-snb) fdo#103925
Test testdisplay:
                pass       -> DMESG-WARN (shard-apl) fdo#104727

fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#104727 https://bugs.freedesktop.org/show_bug.cgi?id=104727

shard-apl        total:2680 pass:1834 dwarn:2   dfail:0   fail:7   skip:836 time:12650s
shard-hsw        total:2680 pass:1781 dwarn:1   dfail:0   fail:6   skip:891 time:11253s
shard-snb        total:2680 pass:1377 dwarn:1   dfail:0   fail:4   skip:1298 time:6834s
Blacklisted hosts:
shard-kbl        total:2680 pass:1961 dwarn:1   dfail:0   fail:8   skip:710 time:9140s

== Logs ==

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

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

[Nautiyal, Ankit K]

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


On 4/6/2018 11:14 PM, Ville Syrjälä wrote:
> On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
>> This patch is causing failure of IGT test kms_3d. The kms_3d test
>> expects the no. of 3d modes to be 13.
>>
>> (The test has hard-coded value for expected no. of 3d modes as 13)
>>
>> But due to the addition of "matching aspect_ratio" in drm_mode_equal in
>> this patch, the total no. of
>>
>> modes in the connector modelist is increased by 2, resulting in failure
>> of assertion 'mode_count==13'.
> If kms_3d isn't setting the aspect ratio cap how is it affected by these
> changes?
In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
to remove duplicate modes in connector_modes from the 
connector->probe_modes.
Earlier, it wasn't matching for aspect-ratio and was discarding two of 
the modes with aspect ratio,
as duplicates of other modes in the list.

Later, when we are pruning the modes in drm_mode_get_connector, the 
logic there assumes,
that the modes are in a sorted order so that we just match with the last 
valid mode for uniqueness.
This isn't the case with the spoofed edid in kms_3d.
Earlier, I was thinking if we should change the no. of expected modes in 
kms_3d,
but that's not correct approach.

So finally, The pruning logic needs to be changed, to do away with any 
assumption and check
all the modes in the list for duplicates. This however will take more 
time to remove duplicates.

Any other suggestions on this?

Regards
-Ankit

>
>> Perhaps this need to be handled in the test.
>>
>> -Regards,
>>
>> Ankit
>>
>>
>> On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
>>> 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;
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>


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

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

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

Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

[Daniel Vetter]

On Tue, Apr 17, 2018 at 10:45:07AM +0530, Nautiyal, Ankit K wrote:
> 
> On 4/6/2018 11:14 PM, Ville Syrjälä wrote:
> > On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
> > > This patch is causing failure of IGT test kms_3d. The kms_3d test
> > > expects the no. of 3d modes to be 13.
> > > 
> > > (The test has hard-coded value for expected no. of 3d modes as 13)
> > > 
> > > But due to the addition of "matching aspect_ratio" in drm_mode_equal in
> > > this patch, the total no. of
> > > 
> > > modes in the connector modelist is increased by 2, resulting in failure
> > > of assertion 'mode_count==13'.
> > If kms_3d isn't setting the aspect ratio cap how is it affected by these
> > changes?
> In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
> to remove duplicate modes in connector_modes from the
> connector->probe_modes.
> Earlier, it wasn't matching for aspect-ratio and was discarding two of the
> modes with aspect ratio,
> as duplicates of other modes in the list.
> 
> Later, when we are pruning the modes in drm_mode_get_connector, the logic
> there assumes,
> that the modes are in a sorted order so that we just match with the last
> valid mode for uniqueness.
> This isn't the case with the spoofed edid in kms_3d.
> Earlier, I was thinking if we should change the no. of expected modes in
> kms_3d,
> but that's not correct approach.
> 
> So finally, The pruning logic needs to be changed, to do away with any
> assumption and check
> all the modes in the list for duplicates. This however will take more time
> to remove duplicates.

Agreed that the best option looks like just fixing the pruning logic.
Since we're talking about very few modes, and in the probe path (where
reading edid takes 20ms easily) the additional cpu overhead really doesn't
matter I think.
-Daniel


> Any other suggestions on this?
> 
> Regards
> -Ankit
> 
> > 
> > > Perhaps this need to be handled in the test.
> > > 
> > > -Regards,
> > > 
> > > Ankit
> > > 
> > > 
> > > On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> > > > 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;
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

On Tue, Apr 17, 2018 at 10:45:07AM +0530, Nautiyal, Ankit K wrote:
> 
> On 4/6/2018 11:14 PM, Ville Syrjälä wrote:
> > On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
> >> This patch is causing failure of IGT test kms_3d. The kms_3d test
> >> expects the no. of 3d modes to be 13.
> >>
> >> (The test has hard-coded value for expected no. of 3d modes as 13)
> >>
> >> But due to the addition of "matching aspect_ratio" in drm_mode_equal in
> >> this patch, the total no. of
> >>
> >> modes in the connector modelist is increased by 2, resulting in failure
> >> of assertion 'mode_count==13'.
> > If kms_3d isn't setting the aspect ratio cap how is it affected by these
> > changes?
> In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
> to remove duplicate modes in connector_modes from the 
> connector->probe_modes.
> Earlier, it wasn't matching for aspect-ratio and was discarding two of 
> the modes with aspect ratio,
> as duplicates of other modes in the list.
> 
> Later, when we are pruning the modes in drm_mode_get_connector, the 
> logic there assumes,
> that the modes are in a sorted order so that we just match with the last 
> valid mode for uniqueness.
> This isn't the case with the spoofed edid in kms_3d.
> Earlier, I was thinking if we should change the no. of expected modes in 
> kms_3d,
> but that's not correct approach.
> 
> So finally, The pruning logic needs to be changed, to do away with any 
> assumption and check
> all the modes in the list for duplicates. This however will take more 
> time to remove duplicates.
> 
> Any other suggestions on this?

What are all the modes this EDID gives us? The order in which the
modes are listed in the EDID should not be relevant as we sort
the mode list ourselves, and thus similar modes should appear back to
back on the list. So I don't really understand how we fail to
discard these two modes.

> 
> Regards
> -Ankit
> 
> >
> >> Perhaps this need to be handled in the test.
> >>
> >> -Regards,
> >>
> >> Ankit
> >>
> >>
> >> On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> >>> 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;
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> 

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

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

[Nautiyal, Ankit K]

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



On 4/17/2018 11:17 PM, Ville Syrjälä wrote:
> On Tue, Apr 17, 2018 at 10:45:07AM +0530, Nautiyal, Ankit K wrote:
>> On 4/6/2018 11:14 PM, Ville Syrjälä wrote:
>>> On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
>>>> This patch is causing failure of IGT test kms_3d. The kms_3d test
>>>> expects the no. of 3d modes to be 13.
>>>>
>>>> (The test has hard-coded value for expected no. of 3d modes as 13)
>>>>
>>>> But due to the addition of "matching aspect_ratio" in drm_mode_equal in
>>>> this patch, the total no. of
>>>>
>>>> modes in the connector modelist is increased by 2, resulting in failure
>>>> of assertion 'mode_count==13'.
>>> If kms_3d isn't setting the aspect ratio cap how is it affected by these
>>> changes?
>> In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
>> to remove duplicate modes in connector_modes from the
>> connector->probe_modes.
>> Earlier, it wasn't matching for aspect-ratio and was discarding two of
>> the modes with aspect ratio,
>> as duplicates of other modes in the list.
>>
>> Later, when we are pruning the modes in drm_mode_get_connector, the
>> logic there assumes,
>> that the modes are in a sorted order so that we just match with the last
>> valid mode for uniqueness.
>> This isn't the case with the spoofed edid in kms_3d.
>> Earlier, I was thinking if we should change the no. of expected modes in
>> kms_3d,
>> but that's not correct approach.
>>
>> So finally, The pruning logic needs to be changed, to do away with any
>> assumption and check
>> all the modes in the list for duplicates. This however will take more
>> time to remove duplicates.
>>
>> Any other suggestions on this?
> What are all the modes this EDID gives us? The order in which the
> modes are listed in the EDID should not be relevant as we sort
> the mode list ourselves, and thus similar modes should appear back to
> back on the list. So I don't really understand how we fail to
> discard these two modes.

As we know the kms_3d test does not set the aspect ratio cap, the modes 
with aspect-ratios must be pruned,
unless they are unique.
Now the list of mode in kms_3d is something like this:
...
1280x720@60 flags 0x1c005 [3D mode - top and bottom] ->1
1280x720@60 flags 0x4005 [3D mode -frame packing] -> 2
1280x720@60 flags 0x1c005 [3D mode -top and bottom with aspect ratio]->3
1280x720@60 flags 0x4005 [3D mode frame packing with aspect ratio] ->4
...

The first two 3D modes are able to pass and are added in the 
drm_mode_get_connector mode list.
For third mode , the 3d mode with aspect ratio, must be pruned if not 
unique. So we check with the last valid mode in the list (1280x720@60 
flags 0x4005).
Everything matches except the 3d mode flags. So it gets added to the list.
For the fourth again, the mode with aspect ratio must be pruned, (not 
unique), but when we check with the last valid mode,
the 3d flags doesn't match, and we add that also in the list.

had the list been sorted by flag also:
1280x720@60 flags 0x1c005 [3D mode top and bottom]
1280x720@60 flags 0x1c005 [3D mode top and bottom with aspect ratio]
1280x720@60 flags 0x4005 [3D mode frame packing]
1280x720@60 flags 0x4005 [3D mode frame packing aspect ratio]

the aspect-ratio modes would have been pruned.

To avoid these scenarios, the best way is to modify the pruning logic to 
check against the whole list of already added modes.
I will send a patch for that shortly.

Regards,
Ankit

>
>> Regards
>> -Ankit
>>
>>>> Perhaps this need to be handled in the test.
>>>>
>>>> -Regards,
>>>>
>>>> Ankit
>>>>
>>>>
>>>> On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
>>>>> 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;
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

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

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