On Wed, Jun 21, 2017 at 04:04:13PM +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 "hdmi_output_format", using which, > a user 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. > > PS: While doing modeset for YCBCR 420 only modes, this property is > ignored, as those timings can be supported only in YCBCR 420 > output mode. > > V2: Added description for the new variable to address build warning > V3: Rebase > V4: Rebase > V5: Added get_property counterpart to fix IGT BAT failures > > 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> Two comments on this: - The kerneldoc for this new property is missing, see https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#kms-properties for what that should look like. I think a new section for HDMI properties might be good. For the text itself just take your commit message and make sure it formats correctly when building the kernel documentation. - Putting a HDMI-specific property into the set of standard properties feels rather wrong, we already have functions to e.g. create tv or dvi-i properties. I think it'd be much better to maybe have a function to create all the HDMI properties. I'd would be really lovely if we could document the other HDMI properties like broadcast mode while at it too. - The property values should be limited to what the driver can support, I guess that would mean limiting the available ycbcr modes? Or does all our hw support all the modes, including 420 (on the sink side)? Thanks, Daniel > --- > drivers/gpu/drm/drm_atomic.c | 4 ++++ > 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, 63 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 77dcef0..ea90f8e 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); > @@ -1272,6 +1274,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, > *val = state->picture_aspect_ratio; > } else if (property == connector->scaling_mode_property) { > *val = state->scaling_mode; > + } else if (property == config->hdmi_output_property) { > + *val = state->hdmi_output; > } else if (connector->funcs->atomic_get_property) { > return connector->funcs->atomic_get_property(connector, > state, property, val); > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 86d3093..1356b3f 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -637,6 +637,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 1305fe9..5ba1f32 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -321,6 +321,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 > @@ -365,6 +376,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; > }; > > /** > @@ -993,6 +1010,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 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
