On Tue, May 30, 2017 at 10:18:19PM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 5/30/2017 10:06 PM, Ville Syrjälä wrote: > > On Tue, May 30, 2017 at 05:43:44PM +0530, Shashank Sharma wrote: > >> HDMI displays can support various output types, based on > >> the color space and subsampling type. The possible > >> outputs from a HDMI 2.0 monitor could be: > >> - RGB > >> - YCBCR 444 > >> - YCBCR 422 > >> - YCBCR 420 > >> > >> This patch adds a drm property, using which, a userspace > > I think we should add the 4:2:0 only mode support first since that > > doesn't require new uapi. The uapi stuff probably needs more careful > > thought as we might want to consider exposing more of the > > colorimetry stuff supported by the HDMI infoframes, and DP MSA MISC > > and VSC SDP. > I was coming from the opposite school of thought. I was thinking > 420_only modes should > be handled carefully, whereas 420_also doesnt need any uapi (This patch > series contains > 420_also and doesnt modify the UAPI) due to following reasons: > > assume there is a mode 3840x2160@60 appears to be in 2 different EDIDs, > in first EDID as a 420_only mode > and in other as 420_also mode. > - If we add a 420_also mode in the mode_list, userspace might pick it > for the modeset as favorite, as most of the > 420 modes are 4k modes. > - Now, if userspace doesn't set the hdmi_output property to YCBCR420, > and sends a modeset on this mode: > - the modeset will be successful in case of a 420_also mode, as it > can be supported in RGB/YUV444 also. > - the modeset will fail in case of 420_only mode, as this mode > cant be supported in any other hdmi output format. > > In this case, addition of 420_only mode, in the connectors->modes list > should be done, only when the driver is ready to > handle the YCBCR420 output, or we have to inform userspace about these > modes which need the hdmi_ouput property to > be set with the modeset, which might in turn need an uabi. > > Does it make a case ? What I think we want is to automagically enable 4:2:0 output if userspace picks a 4:2:0 only mode, regardless of the property value. And in fact this way we don't even need the property to enable the use of 4:2:0 only modes. Which is great because it means current userspace can start to use those modes without any code changes. > - Shashank > >> can specify its preference, for the HDMI output type. > >> The output type enums are similar to the mentioned outputs > >> above. To handle various subsampling of YCBCR output types, > >> this property allows two special cases: > >> - DRM_HDMI_OUTPUT_YCBCR_HQ > >> This indicates preferred output should be YCBCR output, with highest > >> subsampling rate by the source/sink, which can be typically: > >> - ycbcr444 > >> - ycbcr422 > >> - ycbcr420 > >> - DRM_HDMI_OUTPUT_YCBCR_LQ > >> This indicates preferred output should be YCBCR output, with lowest > >> subsampling rate supported by source/sink, which can be: > >> - ycbcr420 > >> - ycbcr422 > >> - ycbcr444 > >> > >> Default value of the property is set to 0 = RGB, so no changes if you > >> dont set the property. > >> > >> V2: Added description for the new variable to address build warning > >> > >> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> > >> Cc: Jose Abreu <joabreu@xxxxxxxxxxxx> > >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx> > >> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx> > >> --- > >> drivers/gpu/drm/drm_atomic.c | 2 ++ > >> drivers/gpu/drm/drm_atomic_helper.c | 4 ++++ > >> drivers/gpu/drm/drm_connector.c | 32 ++++++++++++++++++++++++++++++++ > >> include/drm/drm_connector.h | 18 ++++++++++++++++++ > >> include/drm/drm_mode_config.h | 5 +++++ > >> 5 files changed, 61 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > >> index e163701..8c4e040 100644 > >> --- a/drivers/gpu/drm/drm_atomic.c > >> +++ b/drivers/gpu/drm/drm_atomic.c > >> @@ -1192,6 +1192,8 @@ int drm_atomic_connector_set_property(struct drm_connector *connector, > >> state->picture_aspect_ratio = val; > >> } else if (property == connector->scaling_mode_property) { > >> state->scaling_mode = val; > >> + } else if (property == config->hdmi_output_property) { > >> + state->hdmi_output = val; > >> } else if (connector->funcs->atomic_set_property) { > >> return connector->funcs->atomic_set_property(connector, > >> state, property, val); > >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > >> index 636e561..86b1780 100644 > >> --- a/drivers/gpu/drm/drm_atomic_helper.c > >> +++ b/drivers/gpu/drm/drm_atomic_helper.c > >> @@ -567,6 +567,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, > >> if (old_connector_state->link_status != > >> new_connector_state->link_status) > >> new_crtc_state->connectors_changed = true; > >> + > >> + if (old_connector_state->hdmi_output != > >> + new_connector_state->hdmi_output) > >> + new_crtc_state->connectors_changed = true; > >> } > >> > >> if (funcs->atomic_check) > >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > >> index 5cd61af..f3c5928 100644 > >> --- a/drivers/gpu/drm/drm_connector.c > >> +++ b/drivers/gpu/drm/drm_connector.c > >> @@ -227,6 +227,11 @@ int drm_connector_init(struct drm_device *dev, > >> config->edid_property, > >> 0); > >> > >> + if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL) > >> + drm_object_attach_property(&connector->base, > >> + config->hdmi_output_property, > >> + 0); > >> + > >> drm_object_attach_property(&connector->base, > >> config->dpms_property, 0); > >> > >> @@ -617,6 +622,26 @@ static const struct drm_prop_enum_list drm_link_status_enum_list[] = { > >> }; > >> DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list) > >> > >> +static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = { > >> + { DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" }, > >> + { DRM_HDMI_OUTPUT_YCBCR444, "output_ycbcr444" }, > >> + { DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" }, > >> + { DRM_HDMI_OUTPUT_YCBCR420, "output_ycbcr420" }, > >> + { DRM_HDMI_OUTPUT_YCBCR_HQ, "output_ycbcr_high_subsampling" }, > >> + { DRM_HDMI_OUTPUT_YCBCR_LQ, "output_ycbcr_low_subsampling" }, > >> + { DRM_HDMI_OUTPUT_INVALID, "invalid_output" }, > >> +}; > >> + > >> +/** > >> + * drm_get_hdmi_output_name - return a string for a given hdmi output enum > >> + * @type: enum of output type > >> + */ > >> +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type) > >> +{ > >> + return drm_hdmi_output_enum_list[type].name; > >> +} > >> +EXPORT_SYMBOL(drm_get_hdmi_output_name); > >> + > >> /** > >> * drm_display_info_set_bus_formats - set the supported bus formats > >> * @info: display info to store bus formats in > >> @@ -789,6 +814,13 @@ int drm_connector_create_standard_properties(struct drm_device *dev) > >> return -ENOMEM; > >> dev->mode_config.link_status_property = prop; > >> > >> + prop = drm_property_create_enum(dev, 0, "hdmi_output_format", > >> + drm_hdmi_output_enum_list, > >> + ARRAY_SIZE(drm_hdmi_output_enum_list)); > >> + if (!prop) > >> + return -ENOMEM; > >> + dev->mode_config.hdmi_output_property = prop; > >> + > >> return 0; > >> } > >> > >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > >> index 49cc38c..f77f283 100644 > >> --- a/include/drm/drm_connector.h > >> +++ b/include/drm/drm_connector.h > >> @@ -313,6 +313,17 @@ struct drm_tv_connector_state { > >> unsigned int hue; > >> }; > >> > >> +/* HDMI output pixel format */ > >> +enum drm_hdmi_output_type { > >> + DRM_HDMI_OUTPUT_DEFAULT_RGB, /* default RGB */ > >> + DRM_HDMI_OUTPUT_YCBCR444, /* YCBCR 444 */ > >> + DRM_HDMI_OUTPUT_YCBCR422, /* YCBCR 422 */ > >> + DRM_HDMI_OUTPUT_YCBCR420, /* YCBCR 420 */ > >> + DRM_HDMI_OUTPUT_YCBCR_HQ, /* Highest subsampled YUV */ > >> + DRM_HDMI_OUTPUT_YCBCR_LQ, /* Lowest subsampled YUV */ > >> + DRM_HDMI_OUTPUT_INVALID, /* Guess what ? */ > >> +}; > >> + > >> /** > >> * struct drm_connector_state - mutable connector state > >> * @connector: backpointer to the connector > >> @@ -357,6 +368,12 @@ struct drm_connector_state { > >> * upscaling, mostly used for built-in panels. > >> */ > >> unsigned int scaling_mode; > >> + > >> + /** > >> + * @hdmi_output: Connector property to control the > >> + * HDMI output mode (RGB/YCBCR444/422/420). > >> + */ > >> + enum drm_hdmi_output_type hdmi_output; > >> }; > >> > >> /** > >> @@ -976,6 +993,7 @@ static inline void drm_connector_unreference(struct drm_connector *connector) > >> > >> const char *drm_get_connector_status_name(enum drm_connector_status status); > >> const char *drm_get_subpixel_order_name(enum subpixel_order order); > >> +const char *drm_get_hdmi_output_name(enum drm_hdmi_output_type type); > >> const char *drm_get_dpms_name(int val); > >> const char *drm_get_dvi_i_subconnector_name(int val); > >> const char *drm_get_dvi_i_select_name(int val); > >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > >> index 4298171..1887261 100644 > >> --- a/include/drm/drm_mode_config.h > >> +++ b/include/drm/drm_mode_config.h > >> @@ -740,6 +740,11 @@ struct drm_mode_config { > >> * the position of the output on the host's screen. > >> */ > >> struct drm_property *suggested_y_property; > >> + /** > >> + * @hdmi_output_property: output pixel format from HDMI display > >> + * Default is set for RGB > >> + */ > >> + struct drm_property *hdmi_output_property; > >> > >> /* dumb ioctl parameters */ > >> uint32_t preferred_depth, prefer_shadow; > >> -- > >> 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel