| Message ID | 56cd65c7299ab0b8667fa020c146bdce17eb86a3.1490203284.git.joabreu@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. 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. -Daniel > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > Cc: Carlos Palminha <palminha@synopsys.com> > Cc: dri-devel@lists.freedesktop.org > --- > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@synopsys.com> >> Cc: Carlos Palminha <palminha@synopsys.com> >> Cc: dri-devel@lists.freedesktop.org >> --- >> 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@lists.freedesktop.org >> 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=
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;
Perform sanity checks so that HDMI 2.0+ modes are not exported to drivers or userspace unless asked to. Signed-off-by: Jose Abreu <joabreu@synopsys.com> Cc: Carlos Palminha <palminha@synopsys.com> Cc: dri-devel@lists.freedesktop.org --- 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(-)