[REBASE 2/5] drm: Handle aspect ratio in modeset paths
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Fri Nov 24 09:00:14 UTC 2017
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 at 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 at linux.intel.com>
>> Cc: Shashank Sharma <shashank.sharma at intel.com>
>> Cc: Jose Abreu <jose.abreu at synopsys.com>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at 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
More information about the dri-devel
mailing list