[REBASE 0/5] Aspect ratio support in DRM layer

[REBASE 0/5] Aspect ratio support in DRM layer

[Shashank Sharma]

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, adds a DRM client option for aspect ratio, and loads aspect
ratio flags, only when the client sets this cap. Also, to test this patch series
there is a userspace implementation by Ankit Nautiyal in Wayland/weston layer:
https://patchwork.freedesktop.org/patch/188125/

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 (1):
  drm: Handle aspect ratio in modeset paths

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

aknautiy (2):
  drm: Add DRM client cap for aspect-ratio
  drm: Expose modes with aspect ratio, only if requested

 drivers/gpu/drm/drm_atomic.c    | 40 +++++++++++++++++++++++++++++++-------
 drivers/gpu/drm/drm_connector.c |  7 +++++++
 drivers/gpu/drm/drm_crtc.c      | 19 ++++++++++++++++++
 drivers/gpu/drm/drm_ioctl.c     |  5 +++++
 drivers/gpu/drm/drm_modes.c     | 43 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_atomic.h        |  2 ++
 include/drm/drm_file.h          |  7 +++++++
 include/uapi/drm/drm.h          |  7 +++++++
 include/uapi/drm/drm_mode.h     |  6 ++++++
 9 files changed, 129 insertions(+), 7 deletions(-)

-- 
2.7.4

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

[REBASE 1/5] drm: Add DRM client cap for aspect-ratio

[Shashank Sharma]

From: aknautiy <ankit.k.nautiyal@intel.com>

A new drm client cap is required to enable 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 information in
modes or not.

This patch adds the client cap for aspect-ratio.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 5 +++++
 include/drm/drm_file.h      | 7 +++++++
 include/uapi/drm/drm.h      | 7 +++++++
 3 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 4aafe48..e092550 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -325,6 +325,11 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
 		file_priv->atomic = req->value;
 		file_priv->universal_planes = req->value;
 		break;
+	case DRM_CLIENT_CAP_ASPECT_RATIO:
+		if (req->value > 1)
+			return -EINVAL;
+		file_priv->aspect_ratio_allowed = req->value;
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 0e0c868..6e0e435 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -182,6 +182,13 @@ struct drm_file {
 	unsigned atomic:1;
 
 	/**
+	 * @aspect_ratio_allowed:
+	 *
+	 * True if client has asked to expose the aspect-ratio flags with 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..fe5f531 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 expose aspect ratio flags to userspace.
+ */
+#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

[REBASE 2/5] drm: Handle aspect ratio in modeset paths

[Shashank Sharma]

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

If the user mode does not support aspect-ratio, and requests for
a modeset with aspect ratio flags, then the flag bits reprsenting
aspect ratio must be ignored.

Similarly, if user space doesn't set the aspect ratio client cap,
while preparing a usermode, the aspect-ratio info must not be given
into it.

This patch:
1. adds a new bit field aspect_ratio_allowed in drm_atomic_state,
   which is set only if the aspect-ratio is supported by the user.
2. discards the aspect-ratio info from the user mode while
   preparing kernel mode structure, during modeset, if the
   user does not support aspect ratio.
3. avoids setting the aspect-ratio info in user-mode, while
   converting user-mode from the kernel-mode.

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>
---
 drivers/gpu/drm/drm_atomic.c | 40 +++++++++++++++++++++++++++++++++-------
 drivers/gpu/drm/drm_crtc.c   | 19 +++++++++++++++++++
 include/drm/drm_atomic.h     |  2 ++
 3 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 37445d5..b9743af 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -338,18 +338,30 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
 	state->mode_blob = NULL;
 
 	if (mode) {
-		drm_mode_convert_to_umode(&umode, mode);
+		/*
+		 * If the user has not asked for aspect ratio support,
+		 * take a copy of the 'mode' and set the aspect ratio as
+		 * None. This copy is used to prepare the user-mode with no
+		 * aspect-ratio info.
+		 */
+		struct drm_display_mode ar_mode;
+		struct drm_atomic_state *atomic_state = state->state;
+
+		drm_mode_copy(&ar_mode, mode);
+		if (atomic_state && !atomic_state->allow_aspect_ratio)
+			ar_mode.picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
+		drm_mode_convert_to_umode(&umode, &ar_mode);
 		state->mode_blob =
 			drm_property_create_blob(state->crtc->dev,
-		                                 sizeof(umode),
-		                                 &umode);
+						 sizeof(umode),
+						 &umode);
 		if (IS_ERR(state->mode_blob))
 			return PTR_ERR(state->mode_blob);
 
-		drm_mode_copy(&state->mode, mode);
+		drm_mode_copy(&state->mode, &ar_mode);
 		state->enable = true;
 		DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
-				 mode->name, state);
+				 ar_mode.name, state);
 	} else {
 		memset(&state->mode, 0, sizeof(state->mode));
 		state->enable = false;
@@ -386,10 +398,23 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 	memset(&state->mode, 0, sizeof(state->mode));
 
 	if (blob) {
+		struct drm_mode_modeinfo *ar_umode;
+		struct drm_atomic_state *atomic_state;
+
+		/*
+		 * If the user-space does not ask for aspect-ratio
+		 * the aspect ratio bits in the drmModeModeinfo from user,
+		 * does not mean anything. Therefore these bits must be
+		 * discarded.
+		 */
+		ar_umode = (struct drm_mode_modeinfo *) blob->data;
+		atomic_state = state->state;
+		if (atomic_state && !atomic_state->allow_aspect_ratio)
+			ar_umode->flags &= (~DRM_MODE_FLAG_PIC_AR_MASK);
 		if (blob->length != sizeof(struct drm_mode_modeinfo) ||
 		    drm_mode_convert_umode(&state->mode,
-		                           (const struct drm_mode_modeinfo *)
-		                            blob->data))
+					   (const struct drm_mode_modeinfo *)
+					    ar_umode))
 			return -EINVAL;
 
 		state->mode_blob = drm_property_blob_get(blob);
@@ -2229,6 +2254,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 
 	state->acquire_ctx = &ctx;
 	state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
+	state->allow_aspect_ratio = file_priv->aspect_ratio_allowed;
 
 retry:
 	plane_mask = 0;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f0556e6..a2d34fa 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -425,6 +425,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
 	drm_modeset_lock(&crtc->mutex, NULL);
 	if (crtc->state) {
 		if (crtc->state->enable) {
+			/*
+			 * If the aspect-ratio is not required by the,
+			 * userspace, do not set the aspect-ratio flags.
+			 */
+			if (!file_priv->aspect_ratio_allowed)
+				crtc->state->mode.picture_aspect_ratio =
+					HDMI_PICTURE_ASPECT_NONE;
 			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
 			crtc_resp->mode_valid = 1;
 
@@ -435,6 +442,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
 		crtc_resp->x = crtc->x;
 		crtc_resp->y = crtc->y;
 		if (crtc->enabled) {
+			/*
+			 * If the aspect-ratio is not required by the,
+			 * userspace, do not set the aspect-ratio flags.
+			 */
+			if (!file_priv->aspect_ratio_allowed)
+				crtc->mode.picture_aspect_ratio =
+					HDMI_PICTURE_ASPECT_NONE;
 			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode);
 			crtc_resp->mode_valid = 1;
 
@@ -610,6 +624,11 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 			goto out;
 		}
 
+		/* If the user does not ask for aspect ratio, reset the aspect
+		 * ratio bits in the usermode.
+		 */
+		if (!file_priv->aspect_ratio_allowed)
+			crtc_req->mode.flags &= (~DRM_MODE_FLAG_PIC_AR_MASK);
 		ret = drm_mode_convert_umode(mode, &crtc_req->mode);
 		if (ret) {
 			DRM_DEBUG_KMS("Invalid mode\n");
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 5afd6e3..ae74b2c 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -209,6 +209,7 @@ struct __drm_private_objs_state {
  * @ref: count of all references to this state (will not be freed until zero)
  * @dev: parent DRM device
  * @allow_modeset: allow full modeset
+ * @allow_aspect_ratio: allow the aspect ratio info to be passes to user
  * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics
  * @async_update: hint for asynchronous plane update
  * @planes: pointer to array of structures with per-plane data
@@ -224,6 +225,7 @@ struct drm_atomic_state {
 
 	struct drm_device *dev;
 	bool allow_modeset : 1;
+	bool allow_aspect_ratio : 1;
 	bool legacy_cursor_update : 1;
 	bool async_update : 1;
 	struct __drm_planes_state *planes;
-- 
2.7.4

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

[REBASE 3/5] drm: Expose modes with aspect ratio, only if requested

[Shashank Sharma]

From: aknautiy <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 if user space requested this information or
not.

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

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

Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/drm_connector.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 704fc89..a246bb5 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1285,6 +1285,13 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
 	 */
 	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
 		return false;
+	/*
+	 * If user-space hasn't configured the driver to expose the modes
+	 * with aspect-ratio, don't expose them.
+	 */
+	if (!file_priv->aspect_ratio_allowed &&
+	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE)
+		return false;
 
 	return true;
 }
-- 
2.7.4

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

[REBASE 4/5] drm: Add aspect ratio parsing in DRM layer

[Shashank Sharma]

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>
---
 drivers/gpu/drm/drm_modes.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 4a3f68a..2e8a11e 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1015,6 +1015,7 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
 	    mode1->vsync_end == mode2->vsync_end &&
 	    mode1->vtotal == mode2->vtotal &&
 	    mode1->vscan == mode2->vscan &&
+	    mode1->picture_aspect_ratio == mode2->picture_aspect_ratio &&
 	    (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) ==
 	     (mode2->flags & ~DRM_MODE_FLAG_3D_MASK))
 		return true;
@@ -1549,6 +1550,21 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
 	out->vrefresh = in->vrefresh;
 	out->flags = in->flags;
 	out->type = in->type;
+	out->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
+
+	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;
 }
@@ -1594,6 +1610,21 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
 	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 */
+	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_basic(out);
 	if (out->status != MODE_OK)
 		goto out;
-- 
2.7.4

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

[REBASE 5/5] drm: Add and handle new aspect ratios in DRM layer

[Shashank Sharma]

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>
---
 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 2e8a11e..b9ec35b 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1559,6 +1559,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 DRM_MODE_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;
@@ -1620,6 +1626,12 @@ int drm_mode_convert_umode(struct drm_display_mode *out,
 	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 5597a87..64177d7 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -89,6 +89,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)
@@ -98,6 +100,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)
 
 /* DPMS flags */
 /* bit compatible with the xorg definitions. */
-- 
2.7.4

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

Re: [REBASE 2/5] drm: Handle aspect ratio in modeset paths

[Ville Syrjälä]

On Fri, Nov 17, 2017 at 03:00:29PM +0530, Shashank Sharma wrote:
> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> If the user mode does not support aspect-ratio, and requests for
> a modeset with aspect ratio flags, then the flag bits reprsenting
> aspect ratio must be ignored.

Rejected, not ignored. Rejection should happen when converting
the umode to mode.

And filtering should happen in getcrtc and getblob. The way you're
currently doing things will corrupt the current state, which is
not good.

> Similarly, if user space doesn't set the aspect ratio client cap,
> while preparing a usermode, the aspect-ratio info must not be given
> into it.
> 
> This patch:
> 1. adds a new bit field aspect_ratio_allowed in drm_atomic_state,
>    which is set only if the aspect-ratio is supported by the user.
> 2. discards the aspect-ratio info from the user mode while
>    preparing kernel mode structure, during modeset, if the
>    user does not support aspect ratio.
> 3. avoids setting the aspect-ratio info in user-mode, while
>    converting user-mode from the kernel-mode.
> 
> 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>
> ---
>  drivers/gpu/drm/drm_atomic.c | 40 +++++++++++++++++++++++++++++++++-------
>  drivers/gpu/drm/drm_crtc.c   | 19 +++++++++++++++++++
>  include/drm/drm_atomic.h     |  2 ++
>  3 files changed, 54 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 37445d5..b9743af 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -338,18 +338,30 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
>  	state->mode_blob = NULL;
>  
>  	if (mode) {
> -		drm_mode_convert_to_umode(&umode, mode);
> +		/*
> +		 * If the user has not asked for aspect ratio support,
> +		 * take a copy of the 'mode' and set the aspect ratio as
> +		 * None. This copy is used to prepare the user-mode with no
> +		 * aspect-ratio info.
> +		 */
> +		struct drm_display_mode ar_mode;
> +		struct drm_atomic_state *atomic_state = state->state;
> +
> +		drm_mode_copy(&ar_mode, mode);
> +		if (atomic_state && !atomic_state->allow_aspect_ratio)
> +			ar_mode.picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> +		drm_mode_convert_to_umode(&umode, &ar_mode);
>  		state->mode_blob =
>  			drm_property_create_blob(state->crtc->dev,
> -		                                 sizeof(umode),
> -		                                 &umode);
> +						 sizeof(umode),
> +						 &umode);
>  		if (IS_ERR(state->mode_blob))
>  			return PTR_ERR(state->mode_blob);
>  
> -		drm_mode_copy(&state->mode, mode);
> +		drm_mode_copy(&state->mode, &ar_mode);
>  		state->enable = true;
>  		DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
> -				 mode->name, state);
> +				 ar_mode.name, state);
>  	} else {
>  		memset(&state->mode, 0, sizeof(state->mode));
>  		state->enable = false;
> @@ -386,10 +398,23 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
>  	memset(&state->mode, 0, sizeof(state->mode));
>  
>  	if (blob) {
> +		struct drm_mode_modeinfo *ar_umode;
> +		struct drm_atomic_state *atomic_state;
> +
> +		/*
> +		 * If the user-space does not ask for aspect-ratio
> +		 * the aspect ratio bits in the drmModeModeinfo from user,
> +		 * does not mean anything. Therefore these bits must be
> +		 * discarded.
> +		 */
> +		ar_umode = (struct drm_mode_modeinfo *) blob->data;
> +		atomic_state = state->state;
> +		if (atomic_state && !atomic_state->allow_aspect_ratio)
> +			ar_umode->flags &= (~DRM_MODE_FLAG_PIC_AR_MASK);
>  		if (blob->length != sizeof(struct drm_mode_modeinfo) ||
>  		    drm_mode_convert_umode(&state->mode,
> -		                           (const struct drm_mode_modeinfo *)
> -		                            blob->data))
> +					   (const struct drm_mode_modeinfo *)
> +					    ar_umode))
>  			return -EINVAL;
>  
>  		state->mode_blob = drm_property_blob_get(blob);
> @@ -2229,6 +2254,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  
>  	state->acquire_ctx = &ctx;
>  	state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
> +	state->allow_aspect_ratio = file_priv->aspect_ratio_allowed;
>  
>  retry:
>  	plane_mask = 0;
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f0556e6..a2d34fa 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -425,6 +425,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  	drm_modeset_lock(&crtc->mutex, NULL);
>  	if (crtc->state) {
>  		if (crtc->state->enable) {
> +			/*
> +			 * If the aspect-ratio is not required by the,
> +			 * userspace, do not set the aspect-ratio flags.
> +			 */
> +			if (!file_priv->aspect_ratio_allowed)
> +				crtc->state->mode.picture_aspect_ratio =
> +					HDMI_PICTURE_ASPECT_NONE;
>  			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
>  			crtc_resp->mode_valid = 1;
>  
> @@ -435,6 +442,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  		crtc_resp->x = crtc->x;
>  		crtc_resp->y = crtc->y;
>  		if (crtc->enabled) {
> +			/*
> +			 * If the aspect-ratio is not required by the,
> +			 * userspace, do not set the aspect-ratio flags.
> +			 */
> +			if (!file_priv->aspect_ratio_allowed)
> +				crtc->mode.picture_aspect_ratio =
> +					HDMI_PICTURE_ASPECT_NONE;
>  			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode);
>  			crtc_resp->mode_valid = 1;
>  
> @@ -610,6 +624,11 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>  			goto out;
>  		}
>  
> +		/* If the user does not ask for aspect ratio, reset the aspect
> +		 * ratio bits in the usermode.
> +		 */
> +		if (!file_priv->aspect_ratio_allowed)
> +			crtc_req->mode.flags &= (~DRM_MODE_FLAG_PIC_AR_MASK);
>  		ret = drm_mode_convert_umode(mode, &crtc_req->mode);
>  		if (ret) {
>  			DRM_DEBUG_KMS("Invalid mode\n");
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 5afd6e3..ae74b2c 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -209,6 +209,7 @@ struct __drm_private_objs_state {
>   * @ref: count of all references to this state (will not be freed until zero)
>   * @dev: parent DRM device
>   * @allow_modeset: allow full modeset
> + * @allow_aspect_ratio: allow the aspect ratio info to be passes to user
>   * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics
>   * @async_update: hint for asynchronous plane update
>   * @planes: pointer to array of structures with per-plane data
> @@ -224,6 +225,7 @@ struct drm_atomic_state {
>  
>  	struct drm_device *dev;
>  	bool allow_modeset : 1;
> +	bool allow_aspect_ratio : 1;
>  	bool legacy_cursor_update : 1;
>  	bool async_update : 1;
>  	struct __drm_planes_state *planes;
> -- 
> 2.7.4

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

Re: [REBASE 3/5] drm: Expose modes with aspect ratio, only if requested

[Ville Syrjälä]

On Fri, Nov 17, 2017 at 03:00:30PM +0530, Shashank Sharma wrote:
> From: aknautiy <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 if user space requested this information or
> not.
> 
> This patch prunes the modes with aspect-ratio information, from
> a connector's modelist, if the user-space has not set the aspect
> ratio DRM client cap.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Jose Abreu <jose.abreu@synopsys.com>
> 
> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 704fc89..a246bb5 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1285,6 +1285,13 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
>  	 */
>  	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
>  		return false;
> +	/*
> +	 * If user-space hasn't configured the driver to expose the modes
> +	 * with aspect-ratio, don't expose them.
> +	 */
> +	if (!file_priv->aspect_ratio_allowed &&
> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE)
> +		return false;

I don't think we can just blindly drop the modes. We would have to
expose them with the aspect ratio cleared. That could lead to
duplicates, but I'm thinking that shouldn't be a real problem for
userspace. Having to filteri out the duplicates would certainly
complicate things a bit.

>  
>  	return true;
>  }
> -- 
> 2.7.4

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

Re: [REBASE 2/5] drm: Handle aspect ratio in modeset paths

[Nautiyal, Ankit K]

Hi Ville,

Thanks for the suggestions. Please find my response inline


On 11/21/2017 10:41 PM, Ville Syrjälä wrote:
> On Fri, Nov 17, 2017 at 03:00:29PM +0530, Shashank Sharma wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> If the user mode does not support aspect-ratio, and requests for
>> a modeset with aspect ratio flags, then the flag bits reprsenting
>> aspect ratio must be ignored.
> Rejected, not ignored. Rejection should happen when converting
> the umode to mode.
>
> And filtering should happen in getcrtc and getblob. The way you're
> currently doing things will corrupt the current state, which is
> not good.

Agreed.
The filtering is being done in getcrtc but getblob was missing.
Will add the changes in getblob and send the new patch.

Regards,
Ankit
>> Similarly, if user space doesn't set the aspect ratio client cap,
>> while preparing a usermode, the aspect-ratio info must not be given
>> into it.
>>
>> This patch:
>> 1. adds a new bit field aspect_ratio_allowed in drm_atomic_state,
>>     which is set only if the aspect-ratio is supported by the user.
>> 2. discards the aspect-ratio info from the user mode while
>>     preparing kernel mode structure, during modeset, if the
>>     user does not support aspect ratio.
>> 3. avoids setting the aspect-ratio info in user-mode, while
>>     converting user-mode from the kernel-mode.
>>
>> 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>
>> ---
>>   drivers/gpu/drm/drm_atomic.c | 40 +++++++++++++++++++++++++++++++++-------
>>   drivers/gpu/drm/drm_crtc.c   | 19 +++++++++++++++++++
>>   include/drm/drm_atomic.h     |  2 ++
>>   3 files changed, 54 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 37445d5..b9743af 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -338,18 +338,30 @@ int drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
>>   	state->mode_blob = NULL;
>>   
>>   	if (mode) {
>> -		drm_mode_convert_to_umode(&umode, mode);
>> +		/*
>> +		 * If the user has not asked for aspect ratio support,
>> +		 * take a copy of the 'mode' and set the aspect ratio as
>> +		 * None. This copy is used to prepare the user-mode with no
>> +		 * aspect-ratio info.
>> +		 */
>> +		struct drm_display_mode ar_mode;
>> +		struct drm_atomic_state *atomic_state = state->state;
>> +
>> +		drm_mode_copy(&ar_mode, mode);
>> +		if (atomic_state && !atomic_state->allow_aspect_ratio)
>> +			ar_mode.picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>> +		drm_mode_convert_to_umode(&umode, &ar_mode);
>>   		state->mode_blob =
>>   			drm_property_create_blob(state->crtc->dev,
>> -		                                 sizeof(umode),
>> -		                                 &umode);
>> +						 sizeof(umode),
>> +						 &umode);
>>   		if (IS_ERR(state->mode_blob))
>>   			return PTR_ERR(state->mode_blob);
>>   
>> -		drm_mode_copy(&state->mode, mode);
>> +		drm_mode_copy(&state->mode, &ar_mode);
>>   		state->enable = true;
>>   		DRM_DEBUG_ATOMIC("Set [MODE:%s] for CRTC state %p\n",
>> -				 mode->name, state);
>> +				 ar_mode.name, state);
>>   	} else {
>>   		memset(&state->mode, 0, sizeof(state->mode));
>>   		state->enable = false;
>> @@ -386,10 +398,23 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
>>   	memset(&state->mode, 0, sizeof(state->mode));
>>   
>>   	if (blob) {
>> +		struct drm_mode_modeinfo *ar_umode;
>> +		struct drm_atomic_state *atomic_state;
>> +
>> +		/*
>> +		 * If the user-space does not ask for aspect-ratio
>> +		 * the aspect ratio bits in the drmModeModeinfo from user,
>> +		 * does not mean anything. Therefore these bits must be
>> +		 * discarded.
>> +		 */
>> +		ar_umode = (struct drm_mode_modeinfo *) blob->data;
>> +		atomic_state = state->state;
>> +		if (atomic_state && !atomic_state->allow_aspect_ratio)
>> +			ar_umode->flags &= (~DRM_MODE_FLAG_PIC_AR_MASK);
>>   		if (blob->length != sizeof(struct drm_mode_modeinfo) ||
>>   		    drm_mode_convert_umode(&state->mode,
>> -		                           (const struct drm_mode_modeinfo *)
>> -		                            blob->data))
>> +					   (const struct drm_mode_modeinfo *)
>> +					    ar_umode))
>>   			return -EINVAL;
>>   
>>   		state->mode_blob = drm_property_blob_get(blob);
>> @@ -2229,6 +2254,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>>   
>>   	state->acquire_ctx = &ctx;
>>   	state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
>> +	state->allow_aspect_ratio = file_priv->aspect_ratio_allowed;
>>   
>>   retry:
>>   	plane_mask = 0;
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index f0556e6..a2d34fa 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -425,6 +425,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
>>   	drm_modeset_lock(&crtc->mutex, NULL);
>>   	if (crtc->state) {
>>   		if (crtc->state->enable) {
>> +			/*
>> +			 * If the aspect-ratio is not required by the,
>> +			 * userspace, do not set the aspect-ratio flags.
>> +			 */
>> +			if (!file_priv->aspect_ratio_allowed)
>> +				crtc->state->mode.picture_aspect_ratio =
>> +					HDMI_PICTURE_ASPECT_NONE;
>>   			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
>>   			crtc_resp->mode_valid = 1;
>>   
>> @@ -435,6 +442,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
>>   		crtc_resp->x = crtc->x;
>>   		crtc_resp->y = crtc->y;
>>   		if (crtc->enabled) {
>> +			/*
>> +			 * If the aspect-ratio is not required by the,
>> +			 * userspace, do not set the aspect-ratio flags.
>> +			 */
>> +			if (!file_priv->aspect_ratio_allowed)
>> +				crtc->mode.picture_aspect_ratio =
>> +					HDMI_PICTURE_ASPECT_NONE;
>>   			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode);
>>   			crtc_resp->mode_valid = 1;
>>   
>> @@ -610,6 +624,11 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>>   			goto out;
>>   		}
>>   
>> +		/* If the user does not ask for aspect ratio, reset the aspect
>> +		 * ratio bits in the usermode.
>> +		 */
>> +		if (!file_priv->aspect_ratio_allowed)
>> +			crtc_req->mode.flags &= (~DRM_MODE_FLAG_PIC_AR_MASK);
>>   		ret = drm_mode_convert_umode(mode, &crtc_req->mode);
>>   		if (ret) {
>>   			DRM_DEBUG_KMS("Invalid mode\n");
>> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
>> index 5afd6e3..ae74b2c 100644
>> --- a/include/drm/drm_atomic.h
>> +++ b/include/drm/drm_atomic.h
>> @@ -209,6 +209,7 @@ struct __drm_private_objs_state {
>>    * @ref: count of all references to this state (will not be freed until zero)
>>    * @dev: parent DRM device
>>    * @allow_modeset: allow full modeset
>> + * @allow_aspect_ratio: allow the aspect ratio info to be passes to user
>>    * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics
>>    * @async_update: hint for asynchronous plane update
>>    * @planes: pointer to array of structures with per-plane data
>> @@ -224,6 +225,7 @@ struct drm_atomic_state {
>>   
>>   	struct drm_device *dev;
>>   	bool allow_modeset : 1;
>> +	bool allow_aspect_ratio : 1;
>>   	bool legacy_cursor_update : 1;
>>   	bool async_update : 1;
>>   	struct __drm_planes_state *planes;
>> -- 
>> 2.7.4

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

Re: [REBASE 3/5] drm: Expose modes with aspect ratio, only if requested

[Sharma, Shashank]

Regards

Shashank


On 11/21/2017 10:41 PM, Ville Syrjälä wrote:
> On Fri, Nov 17, 2017 at 03:00:30PM +0530, Shashank Sharma wrote:
>> From: aknautiy <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 if user space requested this information or
>> not.
>>
>> This patch prunes the modes with aspect-ratio information, from
>> a connector's modelist, if the user-space has not set the aspect
>> ratio DRM client cap.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>
>> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/drm_connector.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 704fc89..a246bb5 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1285,6 +1285,13 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
>>   	 */
>>   	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
>>   		return false;
>> +	/*
>> +	 * If user-space hasn't configured the driver to expose the modes
>> +	 * with aspect-ratio, don't expose them.
>> +	 */
>> +	if (!file_priv->aspect_ratio_allowed &&
>> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE)
>> +		return false;
> I don't think we can just blindly drop the modes. We would have to
> expose them with the aspect ratio cleared. That could lead to
> duplicates, but I'm thinking that shouldn't be a real problem for
> userspace. Having to filteri out the duplicates would certainly
> complicate things a bit.
Yes, Agree. Even I was thinking that the right way should be to:
- add a drm_mode_equal_no_aspect function (like 
drm_mode_equal_no_clock_no_stereo).
- clear the aspect ratio information from the mode, when not asked for.
- check the sorted connector->modes list for duplicates for this mode, 
using above function.
     - if mode exists, remove it from the list
     - if not, keep it in the list

Sounds like a plan ?

- Shashank
>>   
>>   	return true;
>>   }
>> -- 
>> 2.7.4

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

Re: [REBASE 3/5] drm: Expose modes with aspect ratio, only if requested

[Ville Syrjälä]

On Fri, Nov 24, 2017 at 02:36:17PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 11/21/2017 10:41 PM, Ville Syrjälä wrote:
> > On Fri, Nov 17, 2017 at 03:00:30PM +0530, Shashank Sharma wrote:
> >> From: aknautiy <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 if user space requested this information or
> >> not.
> >>
> >> This patch prunes the modes with aspect-ratio information, from
> >> a connector's modelist, if the user-space has not set the aspect
> >> ratio DRM client cap.
> >>
> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Cc: Shashank Sharma <shashank.sharma@intel.com>
> >> Cc: Jose Abreu <jose.abreu@synopsys.com>
> >>
> >> Signed-off-by: aknautiy <ankit.k.nautiyal@intel.com>
> >> ---
> >>   drivers/gpu/drm/drm_connector.c | 7 +++++++
> >>   1 file changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> >> index 704fc89..a246bb5 100644
> >> --- a/drivers/gpu/drm/drm_connector.c
> >> +++ b/drivers/gpu/drm/drm_connector.c
> >> @@ -1285,6 +1285,13 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
> >>   	 */
> >>   	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
> >>   		return false;
> >> +	/*
> >> +	 * If user-space hasn't configured the driver to expose the modes
> >> +	 * with aspect-ratio, don't expose them.
> >> +	 */
> >> +	if (!file_priv->aspect_ratio_allowed &&
> >> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE)
> >> +		return false;
> > I don't think we can just blindly drop the modes. We would have to
> > expose them with the aspect ratio cleared. That could lead to
> > duplicates, but I'm thinking that shouldn't be a real problem for
> > userspace. Having to filteri out the duplicates would certainly
> > complicate things a bit.
> Yes, Agree. Even I was thinking that the right way should be to:
> - add a drm_mode_equal_no_aspect function (like 
> drm_mode_equal_no_clock_no_stereo).

Or just drm_mode_match() with the right flags ;)

> - clear the aspect ratio information from the mode, when not asked for.
> - check the sorted connector->modes list for duplicates for this mode, 
> using above function.
>      - if mode exists, remove it from the list
>      - if not, keep it in the list

Hmm. Since the list should be sorted I guess this won't even have to
traverse the list mutliple times. We can just keep skipping modes as
long they match the last mode we've already decided to expose.

> 
> Sounds like a plan ?
> 
> - Shashank
> >>   
> >>   	return true;
> >>   }
> >> -- 
> >> 2.7.4

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