All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v12 00/10] Aspect ratio support in DRM layer
@ 2018-04-27 12:14 Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 01/10] drm/modes: Introduce drm_mode_match() Nautiyal, Ankit K
                   ` (15 more replies)
  0 siblings, 16 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: ppaalanen

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 (3):
  drm: Add DRM client cap for aspect-ratio
  drm: Handle aspect ratio info in legacy modeset path
  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_connector.c |  52 ++++++++--
 drivers/gpu/drm/drm_crtc.c      |   8 ++
 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_modes.c     | 224 ++++++++++++++++++++++++++++++++++------
 drivers/video/hdmi.c            |   3 +
 include/drm/drm_file.h          |   8 ++
 include/drm/drm_modes.h         |  22 ++++
 include/uapi/drm/drm.h          |   7 ++
 include/uapi/drm/drm_mode.h     |   6 ++
 11 files changed, 341 insertions(+), 51 deletions(-)

-- 
2.7.4

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v12 01/10] drm/modes: Introduce drm_mode_match()
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 02/10] drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy Nautiyal, Ankit K
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: ppaalanen

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v12 02/10] drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 01/10] drm/modes: Introduce drm_mode_match() Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 03/10] drm/edid: Fix cea mode aspect ratio handling Nautiyal, Ankit K
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx

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 08d33b4..5475f31 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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v12 03/10] drm/edid: Fix cea mode aspect ratio handling
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 01/10] drm/modes: Introduce drm_mode_match() Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 02/10] drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 04/10] drm/edid: Don't send bogus aspect ratios in AVI infoframes Nautiyal, Ankit K
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: ppaalanen

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 5475f31..3f157c8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2930,11 +2930,15 @@ cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode)
 static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match,
 					     unsigned int clock_tolerance)
 {
+	unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS;
 	u8 vic;
 
 	if (!to_match->clock)
 		return 0;
 
+	if (to_match->picture_aspect_ratio)
+		match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
 	for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
 		struct drm_display_mode cea_mode = edid_cea_modes[vic];
 		unsigned int clock1, clock2;
@@ -2948,7 +2952,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m
 			continue;
 
 		do {
-			if (drm_mode_equal_no_clocks_no_stereo(to_match, &cea_mode))
+			if (drm_mode_match(to_match, &cea_mode, match_flags))
 				return vic;
 		} while (cea_mode_alternate_timings(vic, &cea_mode));
 	}
@@ -2965,11 +2969,15 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m
  */
 u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
 {
+	unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS;
 	u8 vic;
 
 	if (!to_match->clock)
 		return 0;
 
+	if (to_match->picture_aspect_ratio)
+		match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
 	for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
 		struct drm_display_mode cea_mode = edid_cea_modes[vic];
 		unsigned int clock1, clock2;
@@ -2983,7 +2991,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
 			continue;
 
 		do {
-			if (drm_mode_equal_no_clocks_no_stereo(to_match, &cea_mode))
+			if (drm_mode_match(to_match, &cea_mode, match_flags))
 				return vic;
 		} while (cea_mode_alternate_timings(vic, &cea_mode));
 	}
@@ -3030,6 +3038,7 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
 static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match,
 					      unsigned int clock_tolerance)
 {
+	unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS;
 	u8 vic;
 
 	if (!to_match->clock)
@@ -3047,7 +3056,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_
 		    abs(to_match->clock - clock2) > clock_tolerance)
 			continue;
 
-		if (drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
+		if (drm_mode_match(to_match, hdmi_mode, match_flags))
 			return vic;
 	}
 
@@ -3064,6 +3073,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_
  */
 static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
 {
+	unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS;
 	u8 vic;
 
 	if (!to_match->clock)
@@ -3079,7 +3089,7 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
 
 		if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||
 		     KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) &&
-		    drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
+		    drm_mode_match(to_match, hdmi_mode, match_flags))
 			return vic;
 	}
 	return 0;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v12 04/10] drm/edid: Don't send bogus aspect ratios in AVI infoframes
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (2 preceding siblings ...)
  2018-04-27 12:14 ` [PATCH v12 03/10] drm/edid: Fix cea mode aspect ratio handling Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 05/10] video/hdmi: Reject illegal picture aspect ratios Nautiyal, Ankit K
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: ppaalanen

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 3f157c8..40e1e24 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4833,6 +4833,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)
@@ -4875,13 +4876,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 	 * Populate picture aspect ratio from either
 	 * user input (if specified) or from the CEA mode list.
 	 */
-	if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 ||
-		mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9)
-		frame->picture_aspect = mode->picture_aspect_ratio;
-	else if (frame->video_code > 0)
-		frame->picture_aspect = drm_get_cea_aspect_ratio(
-						frame->video_code);
+	picture_aspect = mode->picture_aspect_ratio;
+	if (picture_aspect == HDMI_PICTURE_ASPECT_NONE)
+		picture_aspect = drm_get_cea_aspect_ratio(frame->video_code);
 
+	/*
+	 * The infoframe can't convey anything but none, 4:3
+	 * and 16:9, so if the user has asked for anything else
+	 * we can only satisfy it by specifying the right VIC.
+	 */
+	if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) {
+		if (picture_aspect !=
+		    drm_get_cea_aspect_ratio(frame->video_code))
+			return -EINVAL;
+		picture_aspect = HDMI_PICTURE_ASPECT_NONE;
+	}
+
+	frame->picture_aspect = picture_aspect;
 	frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
 	frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v12 05/10] video/hdmi: Reject illegal picture aspect ratios
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (3 preceding siblings ...)
  2018-04-27 12:14 ` [PATCH v12 04/10] drm/edid: Don't send bogus aspect ratios in AVI infoframes Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 06/10] drm: Add DRM client cap for aspect-ratio Nautiyal, Ankit K
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: ppaalanen

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v12 06/10] drm: Add DRM client cap for aspect-ratio
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (4 preceding siblings ...)
  2018-04-27 12:14 ` [PATCH v12 05/10] video/hdmi: Reject illegal picture aspect ratios Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 07/10] drm: Handle aspect ratio info in legacy modeset path Nautiyal, Ankit K
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx

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
non-atomic 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 non-atomic
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.

Since no atomic-userspaces blow up on receiving aspect-ratio
information, the client cap for aspect-ratio is always enabled
for atomic clients.

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: rebase
V11: rebase
v12: As suggested by Daniel Vetter and Ville Syrjala,
     always enable aspect-ratio client cap for atomic userspaces,
     if no atomic userspace breaks on aspect-ratio 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..0379983 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -324,6 +324,15 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
 			return -EINVAL;
 		file_priv->atomic = req->value;
 		file_priv->universal_planes = req->value;
+		/*
+		 * No atomic user-space blows up on aspect ratio mode bits.
+		 */
+		file_priv->aspect_ratio_allowed = req->value;
+		break;
+	case DRM_CLIENT_CAP_ASPECT_RATIO:
+		if (req->value > 1)
+			return -EINVAL;
+		file_priv->aspect_ratio_allowed = req->value;
 		break;
 	default:
 		return -EINVAL;
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 5176c37..02b7dde 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -182,6 +182,14 @@ struct drm_file {
 	unsigned atomic:1;
 
 	/**
+	 * @aspect_ratio_allowed:
+	 *
+	 * True, if client can handle picture aspect ratios, and has requested
+	 * to pass this information along with the mode.
+	 */
+	unsigned aspect_ratio_allowed:1;
+
+	/**
 	 * @is_master:
 	 *
 	 * This client is the creator of @master. Protected by struct
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 6fdff59..9c660e1 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -680,6 +680,13 @@ struct drm_get_cap {
  */
 #define DRM_CLIENT_CAP_ATOMIC	3
 
+/**
+ * DRM_CLIENT_CAP_ASPECT_RATIO
+ *
+ * If set to 1, the DRM core will provide aspect ratio information in modes.
+ */
+#define DRM_CLIENT_CAP_ASPECT_RATIO    4
+
 /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
 struct drm_set_client_cap {
 	__u64 capability;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v12 07/10] drm: Handle aspect ratio info in legacy modeset path
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (5 preceding siblings ...)
  2018-04-27 12:14 ` [PATCH v12 06/10] drm: Add DRM client cap for aspect-ratio Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 13:54   ` Ville Syrjälä
  2018-04-27 12:14 ` [PATCH v12 08/10] drm: Expose modes with aspect ratio, only if requested Nautiyal, Ankit K
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: ppaalanen

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

If the user-space does not support aspect-ratio, and requests for a
modeset with mode having aspect ratio bits set, then the given
user-mode must be rejected. Secondly, while preparing a user-mode from
kernel mode, the aspect-ratio info must not be given, if aspect-ratio
is not supported by the user.

This patch:
1. rejects the modes with aspect-ratio info, during modeset, if the
   user does not support aspect ratio.
2. does not load the aspect-ratio info in user-mode structure, if
   aspect ratio is not supported.
3. 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>

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
v11: rebase
v12: Merged the patch for adding aspect-ratio helper functions
     with this patch.
---
 drivers/gpu/drm/drm_crtc.c  |  8 ++++++++
 drivers/gpu/drm/drm_modes.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_modes.h     |  4 ++++
 3 files changed, 57 insertions(+)

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_modes.c b/drivers/gpu/drm/drm_modes.c
index c395a24..d6f68c8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1759,3 +1759,48 @@ 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;
+}
+
+/**
+ * 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;
+}
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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v12 08/10] drm: Expose modes with aspect ratio, only if requested
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (6 preceding siblings ...)
  2018-04-27 12:14 ` [PATCH v12 07/10] drm: Handle aspect ratio info in legacy modeset path Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 14:05   ` Ville Syrjälä
  2018-04-27 12:14 ` [PATCH v12 09/10] drm: Add aspect ratio parsing in DRM layer Nautiyal, Ankit K
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx

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

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

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

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

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

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v12 09/10] drm: Add aspect ratio parsing in DRM layer
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (7 preceding siblings ...)
  2018-04-27 12:14 ` [PATCH v12 08/10] drm: Expose modes with aspect ratio, only if requested Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 12:14 ` [PATCH v12 10/10] drm: Add and handle new aspect ratios " Nautiyal, Ankit K
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx

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
V11: rebase
V12: 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 d6f68c8..17a1667 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1049,7 +1049,8 @@ bool drm_mode_equal(const struct drm_display_mode *mode1,
 			      DRM_MODE_MATCH_TIMINGS |
 			      DRM_MODE_MATCH_CLOCK |
 			      DRM_MODE_MATCH_FLAGS |
-			      DRM_MODE_MATCH_3D_FLAGS);
+			      DRM_MODE_MATCH_3D_FLAGS|
+			      DRM_MODE_MATCH_ASPECT_RATIO);
 }
 EXPORT_SYMBOL(drm_mode_equal);
 
@@ -1647,6 +1648,20 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
 	out->vrefresh = in->vrefresh;
 	out->flags = in->flags;
 	out->type = in->type;
+
+	switch (in->picture_aspect_ratio) {
+	case HDMI_PICTURE_ASPECT_4_3:
+		out->flags |= DRM_MODE_FLAG_PIC_AR_4_3;
+		break;
+	case HDMI_PICTURE_ASPECT_16_9:
+		out->flags |= DRM_MODE_FLAG_PIC_AR_16_9;
+		break;
+	case HDMI_PICTURE_ASPECT_RESERVED:
+	default:
+		out->flags |= DRM_MODE_FLAG_PIC_AR_NONE;
+		break;
+	}
+
 	strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
 	out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
 }
@@ -1693,6 +1708,24 @@ int drm_mode_convert_umode(struct drm_device *dev,
 	strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
 	out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
 
+	/* Clearing picture aspect ratio bits from out flags,
+	 * as the aspect-ratio information is not stored in
+	 * flags for kernel-mode, but in picture_aspect_ratio.
+	 */
+	out->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
+
+	switch (in->flags & DRM_MODE_FLAG_PIC_AR_MASK) {
+	case DRM_MODE_FLAG_PIC_AR_4_3:
+		out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3;
+		break;
+	case DRM_MODE_FLAG_PIC_AR_16_9:
+		out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9;
+		break;
+	default:
+		out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+		break;
+	}
+
 	out->status = drm_mode_validate_driver(dev, out);
 	if (out->status != MODE_OK)
 		return -EINVAL;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v12 10/10] drm: Add and handle new aspect ratios in DRM layer
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (8 preceding siblings ...)
  2018-04-27 12:14 ` [PATCH v12 09/10] drm: Add aspect ratio parsing in DRM layer Nautiyal, Ankit K
@ 2018-04-27 12:14 ` Nautiyal, Ankit K
  2018-04-27 12:44 ` ✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support " Patchwork
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-27 12:14 UTC (permalink / raw)
  To: dri-devel, intel-gfx; +Cc: ppaalanen

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
V11: rebase
V12: 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 17a1667..4a04851 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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (9 preceding siblings ...)
  2018-04-27 12:14 ` [PATCH v12 10/10] drm: Add and handle new aspect ratios " Nautiyal, Ankit K
@ 2018-04-27 12:44 ` Patchwork
  2018-04-27 12:59 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-04-27 12:44 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

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

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

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

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

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

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* ✗ Fi.CI.BAT: failure for Aspect ratio support in DRM layer
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (10 preceding siblings ...)
  2018-04-27 12:44 ` ✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support " Patchwork
@ 2018-04-27 12:59 ` Patchwork
  2018-04-27 13:28 ` ✗ Fi.CI.CHECKPATCH: warning " Patchwork
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-04-27 12:59 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4107 -> Patchwork_8824 =

== Summary - FAILURE ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@prime_vgem@basic-fence-flip:
      fi-bwr-2160:        PASS -> FAIL

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
      fi-skl-6770hq:      PASS -> FAIL (fdo#103481)

    
    ==== Possible fixes ====

    igt@kms_busy@basic-flip-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-cnl-psr:         FAIL (fdo#100368) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-cnl-psr:         FAIL (fdo#103481) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927


== Participating hosts (39 -> 36) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4107 -> Patchwork_8824

  CI_DRM_4107: f711c0b36f2382983c964bd69d6c477482e604f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4450: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8824: 1cccf4dbbdca46f5654de3cf3e308ad775fe2a46 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4450: b57600ba58ae0cdbad86826fd653aa0191212f27 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

1cccf4dbbdca drm: Add and handle new aspect ratios in DRM layer
58c770cef648 drm: Add aspect ratio parsing in DRM layer
1ba4ec5d7d45 drm: Expose modes with aspect ratio, only if requested
f94f783b89d9 drm: Handle aspect ratio info in legacy modeset path
dc7e7e1e0257 drm: Add DRM client cap for aspect-ratio
580bb243213b video/hdmi: Reject illegal picture aspect ratios
d6886905af86 drm/edid: Don't send bogus aspect ratios in AVI infoframes
78e1b6d70ed4 drm/edid: Fix cea mode aspect ratio handling
f4fd4583689d drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
6d0e3d322715 drm/modes: Introduce drm_mode_match()

== Logs ==

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (11 preceding siblings ...)
  2018-04-27 12:59 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2018-04-27 13:28 ` Patchwork
  2018-04-27 13:47 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-04-27 13:28 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

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

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

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

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

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

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* ✓ Fi.CI.BAT: success for Aspect ratio support in DRM layer
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (12 preceding siblings ...)
  2018-04-27 13:28 ` ✗ Fi.CI.CHECKPATCH: warning " Patchwork
@ 2018-04-27 13:47 ` Patchwork
  2018-04-27 17:13 ` ✓ Fi.CI.IGT: " Patchwork
  2018-04-27 17:55 ` ✗ Fi.CI.IGT: failure " Patchwork
  15 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-04-27 13:47 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4107 -> Patchwork_8825 =

== Summary - WARNING ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

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

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s3:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS

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

    igt@kms_busy@basic-flip-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-cnl-psr:         FAIL (fdo#100368) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-cnl-psr:         FAIL (fdo#103481) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084


== Participating hosts (39 -> 36) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4107 -> Patchwork_8825

  CI_DRM_4107: f711c0b36f2382983c964bd69d6c477482e604f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4450: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8825: 8355947e81868cb3a9def449f9d56e188272c9a6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4450: b57600ba58ae0cdbad86826fd653aa0191212f27 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

8355947e8186 drm: Add and handle new aspect ratios in DRM layer
1b7fe5d2c8aa drm: Add aspect ratio parsing in DRM layer
2a72ce9d5a9a drm: Expose modes with aspect ratio, only if requested
9955311be9aa drm: Handle aspect ratio info in legacy modeset path
e86d5a0d9ee0 drm: Add DRM client cap for aspect-ratio
2daade285efb video/hdmi: Reject illegal picture aspect ratios
38dcb640b419 drm/edid: Don't send bogus aspect ratios in AVI infoframes
a881ad03fda9 drm/edid: Fix cea mode aspect ratio handling
d13115ecbdc1 drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
26bd319b4abf drm/modes: Introduce drm_mode_match()

== Logs ==

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v12 07/10] drm: Handle aspect ratio info in legacy modeset path
  2018-04-27 12:14 ` [PATCH v12 07/10] drm: Handle aspect ratio info in legacy modeset path Nautiyal, Ankit K
@ 2018-04-27 13:54   ` Ville Syrjälä
  2018-04-30  6:21     ` Nautiyal, Ankit K
  0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2018-04-27 13:54 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx, dri-devel

On Fri, Apr 27, 2018 at 05:44:53PM +0530, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> If the user-space does not support aspect-ratio, and requests for a
> modeset with mode having aspect ratio bits set, then the given
> user-mode must be rejected. Secondly, while preparing a user-mode from
> kernel mode, the aspect-ratio info must not be given, if aspect-ratio
> is not supported by the user.
> 
> This patch:
> 1. rejects the modes with aspect-ratio info, during modeset, if the
>    user does not support aspect ratio.
> 2. does not load the aspect-ratio info in user-mode structure, if
>    aspect ratio is not supported.
> 3. 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>
> 
> 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
> v11: rebase
> v12: Merged the patch for adding aspect-ratio helper functions
>      with this patch.
> ---
>  drivers/gpu/drm/drm_crtc.c  |  8 ++++++++
>  drivers/gpu/drm/drm_modes.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_modes.h     |  4 ++++
>  3 files changed, 57 insertions(+)
> 
> 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_modes.c b/drivers/gpu/drm/drm_modes.c
> index c395a24..d6f68c8 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1759,3 +1759,48 @@ 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;

Still looks funny.

> +}
> +
> +/**
> + * 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;
> +}
> 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);

Since these are no longer exported the prototypes should go into
drivers/gpu/drm/drm_crtc_internal.h

>  
>  struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
>  				      int hdisplay, int vdisplay, int vrefresh,
> -- 
> 2.7.4

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v12 08/10] drm: Expose modes with aspect ratio, only if requested
  2018-04-27 12:14 ` [PATCH v12 08/10] drm: Expose modes with aspect ratio, only if requested Nautiyal, Ankit K
@ 2018-04-27 14:05   ` Ville Syrjälä
  2018-04-30  6:52     ` Nautiyal, Ankit K
  0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2018-04-27 14:05 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx, ppaalanen, dri-devel

On Fri, Apr 27, 2018 at 05:44:54PM +0530, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> We parse the EDID and add all the modes in the connector's modelist.
> This adds CEA modes with aspect ratio information too, 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. However if such a mode is unique in the list, it is
> kept in the list, with aspect-ratio flags reset.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Jose Abreu <jose.abreu@synopsys.com>
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> V3: As suggested by Ville, modified the mechanism of pruning of modes
>     with aspect-ratio, if the aspect-ratio is not supported. Instead
>     of straight away pruning such a mode, the mode is retained with
>     aspect ratio bits set to zero, provided it is unique.
> V4: rebase
> V5: Addressed review comments from Ville:
>     -used a pointer to store last valid mode.
>     -avoided, modifying of picture_aspect_ratio in kernel mode,
>      instead only flags bits of user mode are reset (if aspect-ratio
>      is not supported).
> V6: As suggested by Ville, corrected the mode pruning logic and
>     elaborated the mode pruning logic and the assumptions taken.
> V7: rebase
> V8: rebase
> V9: rebase
> V10: rebase
> V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
>      logic to correctly identify and prune modes with aspect-ratio,
>      if aspect-ratio cap is not set.
> V12: As suggested by Ville, added another list_head in
>      drm_mode_display to traverse the list of exposed modes and
>      avoided duplication of modes.
> ---
>  drivers/gpu/drm/drm_connector.c | 52 ++++++++++++++++++++++++++++++++++-------
>  include/drm/drm_modes.h         |  9 +++++++
>  2 files changed, 53 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b3cde89..e12964b 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1531,15 +1531,35 @@ static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne
>  	return connector->encoder;
>  }
>  
> -static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
> -					 const struct drm_file *file_priv)
> +static bool
> +drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
> +			     const struct list_head *modelist,
> +			     const struct drm_file *file_priv)
>  {
>  	/*
>  	 * If user-space hasn't configured the driver to expose the stereo 3D
>  	 * modes, don't expose them.
>  	 */
> +	struct drm_display_mode *mode_itr;
> +

This is clearly misplaced. It should be before the comment. Hmm, 
atually better move it into the aspect ratio if block since it's
not needed elsewhere.

>  	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
>  		return false;
> +	/*
> +	 * If user-space hasn't configured the driver to expose the modes
> +	 * with aspect-ratio, don't expose them. However if such a mode
> +	 * is unique, let it be exposed, but reset the aspect-ratio flags
> +	 * while preparing the list of user-modes.
> +	 */
> +	if (!file_priv->aspect_ratio_allowed &&
> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
> +		list_for_each_entry(mode_itr, modelist, exp_head)
> +			if (drm_mode_match(mode_itr, mode,
> +					   DRM_MODE_MATCH_TIMINGS |
> +					   DRM_MODE_MATCH_CLOCK |
> +					   DRM_MODE_MATCH_FLAGS |
> +					   DRM_MODE_MATCH_3D_FLAGS))
> +				return false;
> +	}
>  
>  	return true;
>  }
> @@ -1550,7 +1570,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	struct drm_mode_get_connector *out_resp = data;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> -	struct drm_display_mode *mode;
> +	struct drm_display_mode *mode, *tmp;
> +	struct list_head modelist;

LIST_HEAD_INIT(modelist);

And maybe call it export_list or something like that.

>  	int mode_count = 0;
>  	int encoders_count = 0;
>  	int ret = 0;
> @@ -1605,23 +1626,34 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	out_resp->subpixel = connector->display_info.subpixel_order;
>  	out_resp->connection = connector->status;
>  
> +	INIT_LIST_HEAD(&modelist);
> +
>  	/* delayed so we get modes regardless of pre-fill_modes state */
>  	list_for_each_entry(mode, &connector->modes, head)
> -		if (drm_mode_expose_to_userspace(mode, file_priv))
> +		if (drm_mode_expose_to_userspace(mode, &modelist,
> +						 file_priv)) {
> +			list_add_tail(&mode->exp_head, &modelist);
>  			mode_count++;
> +		}
>  
>  	/*
>  	 * This ioctl is called twice, once to determine how much space is
>  	 * needed, and the 2nd time to fill it.
> +	 * The modes that need to be exposed to the user are maintained in the
> +	 * 'modelist'. When the ioctl is called first time to determine the,
> +	 * space, the modelist gets filled, to find the no. of modes. In the
> +	 * 2nd time, the user modes are filled, one by one from the modelist.
>  	 */
>  	if ((out_resp->count_modes >= mode_count) && mode_count) {
>  		copied = 0;
>  		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
> -		list_for_each_entry(mode, &connector->modes, head) {
> -			if (!drm_mode_expose_to_userspace(mode, file_priv))
> -				continue;
> -
> +		list_for_each_entry(mode, &modelist, exp_head) {
>  			drm_mode_convert_to_umode(&u_mode, mode);
> +			/*
> +			 * Reset aspect ratio flags of user-mode, if modes with
> +			 * aspect-ratio are not supported.
> +			 */
> +			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
>  			if (copy_to_user(mode_ptr + copied,
>  					 &u_mode, sizeof(u_mode))) {
>  				ret = -EFAULT;
> @@ -1651,6 +1683,10 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>  
>  out:
> +	list_for_each_entry_safe(mode, tmp, &modelist, exp_head)
> +		list_del(&mode->exp_head);
> +	list_del(&modelist);

That last list_del() doesn't make sense. Also you can't touch the
mode->exp_head without locking. We should document that it's
protected by mode_config.mutex.

Since we don't actually care about exp_head outside of this function
I think we can just leave the stale pointers in place. No one else is
supposed to look at them, a fact we should probaly also include in the
documentation.

> +
>  	drm_connector_put(connector);
>  
>  	return ret;
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index e0b060d..1888ec7 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -411,6 +411,15 @@ struct drm_display_mode {
>  	 * Field for setting the HDMI picture aspect ratio of a mode.
>  	 */
>  	enum hdmi_picture_aspect picture_aspect_ratio;
> +
> +	/**
> +	 * @exp_head:

I think we can afford to write 'export_head'.

> +	 *
> +	 * struct list_head for modes to be exposed to the userspace.
> +	 *
> +	 */
> +	struct list_head exp_head;
> +
>  };
>  
>  /**
> -- 
> 2.7.4

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* ✓ Fi.CI.IGT: success for Aspect ratio support in DRM layer
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (13 preceding siblings ...)
  2018-04-27 13:47 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-27 17:13 ` Patchwork
  2018-04-27 17:55 ` ✗ Fi.CI.IGT: failure " Patchwork
  15 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-04-27 17:13 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4107_full -> Patchwork_8824_full =

== Summary - WARNING ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@kms_draw_crc@fill-fb:
      shard-snb:          PASS -> SKIP +2

    igt@kms_flip@flip-vs-fences:
      shard-kbl:          PASS -> SKIP +36

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_softpin@noreloc-s3:
      shard-kbl:          PASS -> DMESG-WARN (fdo#103558)

    igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
      shard-kbl:          PASS -> DMESG-WARN (fdo#103558, fdo#105602) +6

    igt@kms_flip@2x-dpms-vs-vblank-race:
      shard-hsw:          PASS -> FAIL (fdo#103060)

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

    
    ==== Possible fixes ====

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
      shard-hsw:          FAIL (fdo#105767) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@blocking-wf_vblank:
      shard-hsw:          FAIL (fdo#103928) -> PASS

    igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS

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

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

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (9 -> 8) ==

  Missing    (1): shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4107 -> Patchwork_8824

  CI_DRM_4107: f711c0b36f2382983c964bd69d6c477482e604f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4450: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8824: 1cccf4dbbdca46f5654de3cf3e308ad775fe2a46 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4450: b57600ba58ae0cdbad86826fd653aa0191212f27 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* ✗ Fi.CI.IGT: failure for Aspect ratio support in DRM layer
  2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
                   ` (14 preceding siblings ...)
  2018-04-27 17:13 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-04-27 17:55 ` Patchwork
  15 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-04-27 17:55 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4107_full -> Patchwork_8825_full =

== Summary - FAILURE ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled:
      shard-kbl:          PASS -> FAIL +1

    igt@perf@buffer-fill:
      shard-kbl:          PASS -> DMESG-FAIL

    
    ==== Warnings ====

    igt@perf_pmu@cpu-hotplug:
      shard-kbl:          PASS -> SKIP +14

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

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

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

    igt@kms_flip@flip-vs-wf_vblank-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103928)

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

    igt@kms_flip@plain-flip-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368) +2

    
    ==== Possible fixes ====

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
      shard-hsw:          FAIL (fdo#105767) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@blocking-wf_vblank:
      shard-hsw:          FAIL (fdo#103928) -> PASS

    igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS

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

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087


== Participating hosts (9 -> 8) ==

  Missing    (1): shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4107 -> Patchwork_8825

  CI_DRM_4107: f711c0b36f2382983c964bd69d6c477482e604f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4450: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8825: 8355947e81868cb3a9def449f9d56e188272c9a6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4450: b57600ba58ae0cdbad86826fd653aa0191212f27 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v12 07/10] drm: Handle aspect ratio info in legacy modeset path
  2018-04-27 13:54   ` Ville Syrjälä
@ 2018-04-30  6:21     ` Nautiyal, Ankit K
  0 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-30  6:21 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, ppaalanen, dri-devel


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


On 4/27/2018 7:24 PM, Ville Syrjälä wrote:
> On Fri, Apr 27, 2018 at 05:44:53PM +0530, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> If the user-space does not support aspect-ratio, and requests for a
>> modeset with mode having aspect ratio bits set, then the given
>> user-mode must be rejected. Secondly, while preparing a user-mode from
>> kernel mode, the aspect-ratio info must not be given, if aspect-ratio
>> is not supported by the user.
>>
>> This patch:
>> 1. rejects the modes with aspect-ratio info, during modeset, if the
>>     user does not support aspect ratio.
>> 2. does not load the aspect-ratio info in user-mode structure, if
>>     aspect ratio is not supported.
>> 3. 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>
>>
>> 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
>> v11: rebase
>> v12: Merged the patch for adding aspect-ratio helper functions
>>       with this patch.
>> ---
>>   drivers/gpu/drm/drm_crtc.c  |  8 ++++++++
>>   drivers/gpu/drm/drm_modes.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_modes.h     |  4 ++++
>>   3 files changed, 57 insertions(+)
>>
>> 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_modes.c b/drivers/gpu/drm/drm_modes.c
>> index c395a24..d6f68c8 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -1759,3 +1759,48 @@ 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;
> Still looks funny.

Alright. Let me change it to:
   if (file_priv->aspect_ratio_allowed ||
             (umode->flags & DRM_MODE_FLAG_PIC_AR_MASK) == 
DRM_MODE_FLAG_PIC_AR_NONE)
                 return true;
         return false;

>> +}
>> +
>> +/**
>> + * 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;
>> +}
>> 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);
> Since these are no longer exported the prototypes should go into
> drivers/gpu/drm/drm_crtc_internal.h

Got it, will move the prototypes to the correct place.
Thanks for pointing that out.

Regards,
Ankit


>
>>   struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
>>                                      int hdisplay, int vdisplay, int vrefresh,
>> --
>> 2.7.4
> --
> Ville Syrjälä
> Intel


[-- Attachment #1.2: Type: text/html, Size: 7252 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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v12 08/10] drm: Expose modes with aspect ratio, only if requested
  2018-04-27 14:05   ` Ville Syrjälä
@ 2018-04-30  6:52     ` Nautiyal, Ankit K
  0 siblings, 0 replies; 24+ messages in thread
From: Nautiyal, Ankit K @ 2018-04-30  6:52 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, ppaalanen, dri-devel


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


On 4/27/2018 7:35 PM, Ville Syrjälä wrote:
> On Fri, Apr 27, 2018 at 05:44:54PM +0530, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> We parse the EDID and add all the modes in the connector's modelist.
>> This adds CEA modes with aspect ratio information too, 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. However if such a mode is unique in the list, it is
>> kept in the list, with aspect-ratio flags reset.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> V3: As suggested by Ville, modified the mechanism of pruning of modes
>>      with aspect-ratio, if the aspect-ratio is not supported. Instead
>>      of straight away pruning such a mode, the mode is retained with
>>      aspect ratio bits set to zero, provided it is unique.
>> V4: rebase
>> V5: Addressed review comments from Ville:
>>      -used a pointer to store last valid mode.
>>      -avoided, modifying of picture_aspect_ratio in kernel mode,
>>       instead only flags bits of user mode are reset (if aspect-ratio
>>       is not supported).
>> V6: As suggested by Ville, corrected the mode pruning logic and
>>      elaborated the mode pruning logic and the assumptions taken.
>> V7: rebase
>> V8: rebase
>> V9: rebase
>> V10: rebase
>> V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
>>       logic to correctly identify and prune modes with aspect-ratio,
>>       if aspect-ratio cap is not set.
>> V12: As suggested by Ville, added another list_head in
>>       drm_mode_display to traverse the list of exposed modes and
>>       avoided duplication of modes.
>> ---
>>   drivers/gpu/drm/drm_connector.c | 52 ++++++++++++++++++++++++++++++++++-------
>>   include/drm/drm_modes.h         |  9 +++++++
>>   2 files changed, 53 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index b3cde89..e12964b 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1531,15 +1531,35 @@ static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne
>>   	return connector->encoder;
>>   }
>>   
>> -static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
>> -					 const struct drm_file *file_priv)
>> +static bool
>> +drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
>> +			     const struct list_head *modelist,
>> +			     const struct drm_file *file_priv)
>>   {
>>   	/*
>>   	 * If user-space hasn't configured the driver to expose the stereo 3D
>>   	 * modes, don't expose them.
>>   	 */
>> +	struct drm_display_mode *mode_itr;
>> +
> This is clearly misplaced. It should be before the comment. Hmm,
> atually better move it into the aspect ratio if block since it's
> not needed elsewhere.

That makes sense, will move it to the aspect-ratio block.

>
>>   	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
>>   		return false;
>> +	/*
>> +	 * If user-space hasn't configured the driver to expose the modes
>> +	 * with aspect-ratio, don't expose them. However if such a mode
>> +	 * is unique, let it be exposed, but reset the aspect-ratio flags
>> +	 * while preparing the list of user-modes.
>> +	 */
>> +	if (!file_priv->aspect_ratio_allowed &&
>> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
>> +		list_for_each_entry(mode_itr, modelist, exp_head)
>> +			if (drm_mode_match(mode_itr, mode,
>> +					   DRM_MODE_MATCH_TIMINGS |
>> +					   DRM_MODE_MATCH_CLOCK |
>> +					   DRM_MODE_MATCH_FLAGS |
>> +					   DRM_MODE_MATCH_3D_FLAGS))
>> +				return false;
>> +	}
>>   
>>   	return true;
>>   }
>> @@ -1550,7 +1570,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>>   	struct drm_mode_get_connector *out_resp = data;
>>   	struct drm_connector *connector;
>>   	struct drm_encoder *encoder;
>> -	struct drm_display_mode *mode;
>> +	struct drm_display_mode *mode, *tmp;
>> +	struct list_head modelist;
> LIST_HEAD_INIT(modelist);
>
> And maybe call it export_list or something like that.

Alright. Will change the name to export_list.

I hope you mean LIST_HEAD(export_list);

>>   	int mode_count = 0;
>>   	int encoders_count = 0;
>>   	int ret = 0;
>> @@ -1605,23 +1626,34 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>>   	out_resp->subpixel = connector->display_info.subpixel_order;
>>   	out_resp->connection = connector->status;
>>   
>> +	INIT_LIST_HEAD(&modelist);
>> +
>>   	/* delayed so we get modes regardless of pre-fill_modes state */
>>   	list_for_each_entry(mode, &connector->modes, head)
>> -		if (drm_mode_expose_to_userspace(mode, file_priv))
>> +		if (drm_mode_expose_to_userspace(mode, &modelist,
>> +						 file_priv)) {
>> +			list_add_tail(&mode->exp_head, &modelist);
>>   			mode_count++;
>> +		}
>>   
>>   	/*
>>   	 * This ioctl is called twice, once to determine how much space is
>>   	 * needed, and the 2nd time to fill it.
>> +	 * The modes that need to be exposed to the user are maintained in the
>> +	 * 'modelist'. When the ioctl is called first time to determine the,
>> +	 * space, the modelist gets filled, to find the no. of modes. In the
>> +	 * 2nd time, the user modes are filled, one by one from the modelist.
>>   	 */
>>   	if ((out_resp->count_modes >= mode_count) && mode_count) {
>>   		copied = 0;
>>   		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
>> -		list_for_each_entry(mode, &connector->modes, head) {
>> -			if (!drm_mode_expose_to_userspace(mode, file_priv))
>> -				continue;
>> -
>> +		list_for_each_entry(mode, &modelist, exp_head) {
>>   			drm_mode_convert_to_umode(&u_mode, mode);
>> +			/*
>> +			 * Reset aspect ratio flags of user-mode, if modes with
>> +			 * aspect-ratio are not supported.
>> +			 */
>> +			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
>>   			if (copy_to_user(mode_ptr + copied,
>>   					 &u_mode, sizeof(u_mode))) {
>>   				ret = -EFAULT;
>> @@ -1651,6 +1683,10 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>>   	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>   
>>   out:
>> +	list_for_each_entry_safe(mode, tmp, &modelist, exp_head)
>> +		list_del(&mode->exp_head);
>> +	list_del(&modelist);
> That last list_del() doesn't make sense. Also you can't touch the
> mode->exp_head without locking. We should document that it's
> protected by mode_config.mutex.
Oh yes, list_del () is not required, the deletion of the last mode, will 
automatically
set the next and prev of the list_head to itself.
> Since we don't actually care about exp_head outside of this function
> I think we can just leave the stale pointers in place. No one else is
> supposed to look at them, a fact we should probaly also include in the
> documentation.

Ok, so I will let the stale pointers in place, and explain the reasoning 
as comments.
>> +
>>   	drm_connector_put(connector);
>>   
>>   	return ret;
>> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
>> index e0b060d..1888ec7 100644
>> --- a/include/drm/drm_modes.h
>> +++ b/include/drm/drm_modes.h
>> @@ -411,6 +411,15 @@ struct drm_display_mode {
>>   	 * Field for setting the HDMI picture aspect ratio of a mode.
>>   	 */
>>   	enum hdmi_picture_aspect picture_aspect_ratio;
>> +
>> +	/**
>> +	 * @exp_head:
> I think we can afford to write 'export_head'.

Yeah right, shortening export_head to exp_head didn't do much :)
Will change it to export_head.

Regards,
Ankit

>> +	 *
>> +	 * struct list_head for modes to be exposed to the userspace.
>> +	 *
>> +	 */
>> +	struct list_head exp_head;
>> +
>>   };
>>   
>>   /**
>> -- 
>> 2.7.4


[-- Attachment #1.2: Type: text/html, Size: 9713 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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer
  2018-05-08 11:09 [PATCH v14 00/10] " Nautiyal, Ankit K
@ 2018-05-08 11:31 ` Patchwork
  0 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-05-08 11:31 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

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

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

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

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

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

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

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer
  2018-05-02  6:50 [PATCH v13 00/10] " Nautiyal, Ankit K
@ 2018-05-02 10:00 ` Patchwork
  0 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-05-02 10:00 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

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

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

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

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

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

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support in DRM layer
  2018-04-20 13:31 [PATCH v11 00/11] " Nautiyal, Ankit K
@ 2018-04-20 16:34 ` Patchwork
  0 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2018-04-20 16:34 UTC (permalink / raw)
  To: Nautiyal, Ankit K; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

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

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

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

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

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

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

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2018-05-08 11:31 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-27 12:14 [PATCH v12 00/10] Aspect ratio support in DRM layer Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 01/10] drm/modes: Introduce drm_mode_match() Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 02/10] drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 03/10] drm/edid: Fix cea mode aspect ratio handling Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 04/10] drm/edid: Don't send bogus aspect ratios in AVI infoframes Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 05/10] video/hdmi: Reject illegal picture aspect ratios Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 06/10] drm: Add DRM client cap for aspect-ratio Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 07/10] drm: Handle aspect ratio info in legacy modeset path Nautiyal, Ankit K
2018-04-27 13:54   ` Ville Syrjälä
2018-04-30  6:21     ` Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 08/10] drm: Expose modes with aspect ratio, only if requested Nautiyal, Ankit K
2018-04-27 14:05   ` Ville Syrjälä
2018-04-30  6:52     ` Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 09/10] drm: Add aspect ratio parsing in DRM layer Nautiyal, Ankit K
2018-04-27 12:14 ` [PATCH v12 10/10] drm: Add and handle new aspect ratios " Nautiyal, Ankit K
2018-04-27 12:44 ` ✗ Fi.CI.CHECKPATCH: warning for Aspect ratio support " Patchwork
2018-04-27 12:59 ` ✗ Fi.CI.BAT: failure " Patchwork
2018-04-27 13:28 ` ✗ Fi.CI.CHECKPATCH: warning " Patchwork
2018-04-27 13:47 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-27 17:13 ` ✓ Fi.CI.IGT: " Patchwork
2018-04-27 17:55 ` ✗ Fi.CI.IGT: failure " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2018-05-08 11:09 [PATCH v14 00/10] " Nautiyal, Ankit K
2018-05-08 11:31 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-05-02  6:50 [PATCH v13 00/10] " Nautiyal, Ankit K
2018-05-02 10:00 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-04-20 13:31 [PATCH v11 00/11] " Nautiyal, Ankit K
2018-04-20 16:34 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.