[Intel-gfx] [PATCH v2 05/11] drm: create hdmi output property
Sharma, Shashank
shashank.sharma at intel.com
Wed May 31 13:04:08 UTC 2017
Regards
Shashank
On 5/31/2017 6:16 PM, Ville Syrjälä wrote:
> 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 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 ?
> 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.
I was worried about this sitution:
- There is one 420_only mode (38x21 at 60) in EDID
- We added a 420_only mode in the list, which userspace can see (but
cant differentiate between non-420 or 420 mode)
- Userspace triggers a modeset on this, but the driver doesn't support
YCBCR420 output yet, as source doesn't support it yet.
In this case, we will cause modeset failure for each driver which doesnt
support HDMI 2.0.
So if we have to make a 420_only mode visible to userspace, we have to
indicate somehow that its 420_only mode.
Do you think so ?
- Shashank
>> - 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 Intel-gfx
mailing list