[PATCH v2 05/11] drm: create hdmi output property
Sharma, Shashank
shashank.sharma at intel.com
Tue May 30 16:48:19 UTC 2017
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 at 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 ?
- 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 at linux.intel.com>
>> Cc: Jose Abreu <joabreu at synopsys.com>
>> Cc: Daniel Vetter <daniel.vetter at intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>> ---
>> 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
More information about the dri-devel
mailing list