Hi Daniel,
Thanks for the feedback!
On 23-03-2017 07:25, Daniel Vetter wrote:
> On Wed, Mar 22, 2017 at 05:36:01PM +0000, Jose Abreu wrote:
>> Perform sanity checks so that HDMI 2.0+ modes are not exported to
>> drivers or userspace unless asked to.
> Why? They're just normal modes, this doesn't make sense. If you want to
> filter the aspect ratio stuff, that makes sense, but exposing the modes
> with default aspect ratio by default should be doable.
Yes my initial intention was to hide the new aspect ratio modes
from drivers so that only HDMI 1.4 modes will be available to non
HDMI 2.0+ drivers. I think that we should avoid exposing these
new modes to drivers. They have higher pixel clock values and
sometimes EDID can declare that some modes should use a specific
colorspace (YCbCr 4:2:0), but this is another story.
My argument is not very strong though: Pixel clock must always be
checked by drivers and rejected if not supported as well as
aspect ratio flags. Do you know of any driver that does not do this?
>
> I'm also not sure how exactly this is supposed to interact with the
> partially reverted aspect ratio filtering from Shashank. I guess what you
> really want is the feature flag for exposing aspect ratio, and then filter
> accordingly.
So you propose to change this validation to only check for aspect
ratio flags? Shouldn't we play it safe and allow this flag so
that if anything besides aspect ratio arises we can safely
prevent drivers from getting the mode?
Notice that this flag is not for HDMI 2.0 modes only, it is for
HDMI 2.0+. I'm thinking about HDMI 2.1 which will include new
video modes and possible even new flags.
Best regards,
Jose Miguel Abreu
> -Daniel
>> Signed-off-by: Jose Abreu <joabreu@xxxxxxxxxxxx>
>> Cc: Carlos Palminha <palminha@xxxxxxxxxxxx>
>> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> ---
>> drivers/gpu/drm/drm_connector.c | 2 ++
>> drivers/gpu/drm/drm_probe_helper.c | 9 ++++++++-
>> include/drm/drm_modes.h | 14 ++++++++++++++
>> 3 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 9f84761..430bb2b 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1200,6 +1200,8 @@ 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 (!file_priv->hdmi2_allowed && drm_mode_is_hdmi2(mode))
>> + return false;
>>
>> return true;
>> }
>> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
>> index 85005d5..ddacb5d 100644
>> --- a/drivers/gpu/drm/drm_probe_helper.c
>> +++ b/drivers/gpu/drm/drm_probe_helper.c
>> @@ -77,6 +77,10 @@
>> !(flags & DRM_MODE_FLAG_3D_MASK))
>> return MODE_NO_STEREO;
>>
>> + if ((mode->flags & DRM_MODE_FLAG_HDMI2) &&
>> + !(flags & DRM_MODE_FLAG_HDMI2))
>> + return MODE_NO_HDMI2;
>> +
>> return MODE_OK;
>> }
>>
>> @@ -221,7 +225,8 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
>> * - drm_mode_validate_size() filters out modes larger than @maxX and @maxY
>> * (if specified)
>> * - drm_mode_validate_flag() checks the modes against basic connector
>> - * capabilities (interlace_allowed,doublescan_allowed,stereo_allowed)
>> + * capabilities (interlace_allowed,doublescan_allowed,stereo_allowed,
>> + * hdmi2_allowed)
>> * - the optional &drm_connector_helper_funcs.mode_valid helper can perform
>> * driver and/or hardware specific checks
>> *
>> @@ -336,6 +341,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>> mode_flags |= DRM_MODE_FLAG_DBLSCAN;
>> if (connector->stereo_allowed)
>> mode_flags |= DRM_MODE_FLAG_3D_MASK;
>> + if (connector->hdmi2_allowed)
>> + mode_flags |= DRM_MODE_FLAG_HDMI2;
>>
>> list_for_each_entry(mode, &connector->modes, head) {
>> if (mode->status == MODE_OK)
>> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
>> index 6dd34280..83466ff 100644
>> --- a/include/drm/drm_modes.h
>> +++ b/include/drm/drm_modes.h
>> @@ -80,6 +80,7 @@
>> * @MODE_ONE_SIZE: only one resolution is supported
>> * @MODE_NO_REDUCED: monitor doesn't accept reduced blanking
>> * @MODE_NO_STEREO: stereo modes not supported
>> + * @MODE_NO_HDMI2: HDMI 2.0+ modes not supported
>> * @MODE_STALE: mode has become stale
>> * @MODE_BAD: unspecified reason
>> * @MODE_ERROR: error condition
>> @@ -124,6 +125,7 @@ enum drm_mode_status {
>> MODE_ONE_SIZE,
>> MODE_NO_REDUCED,
>> MODE_NO_STEREO,
>> + MODE_NO_HDMI2,
>> MODE_STALE = -3,
>> MODE_BAD = -2,
>> MODE_ERROR = -1
>> @@ -422,6 +424,18 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode)
>> return mode->flags & DRM_MODE_FLAG_3D_MASK;
>> }
>>
>> +/**
>> + * drm_mode_is_hdmi2 - check for HDMI 2.0+ mode flag
>> + * @mode: drm_display_mode to check
>> + *
>> + * Returns:
>> + * True if the mode is HDMI 2.0+ mode, false if not
>> + */
>> +static inline bool drm_mode_is_hdmi2(const struct drm_display_mode *mode)
>> +{
>> + return mode->flags & DRM_MODE_FLAG_HDMI2;
>> +}
>> +
>> struct drm_connector;
>> struct drm_cmdline_mode;
>>
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=ZrB1FUUWlSj6zYGBcO_CFTd1XHdKAzijVyu1XqZ5gfc&s=fMJ9nzD0WR6AhnAOZug1j9qT7yb0_i3tu8BC_D6qiGs&e=
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
