[PATCH 05/17] drm/msm/dp: add an API to indicate if sink supports VSC SDP
Abhinav Kumar
quic_abhinavk at quicinc.com
Sat Jan 27 03:57:33 UTC 2024
On 1/26/2024 6:40 PM, Dmitry Baryshkov wrote:
> On Sat, 27 Jan 2024 at 02:58, Paloma Arellano <quic_parellan at quicinc.com> wrote:
>>
>>
>> On 1/25/2024 1:23 PM, Dmitry Baryshkov wrote:
>>> On 25/01/2024 21:38, Paloma Arellano wrote:
>>>> YUV420 format is supported only in the VSC SDP packet and not through
>>>> MSA. Hence add an API which indicates the sink support which can be used
>>>> by the rest of the DP programming.
>>>
>>> This API ideally should go to drm/display/drm_dp_helper.c
>> I'm not familiar how other vendors are checking if VSC SDP is supported.
>> So in moving this API, I'm going to let the other vendors make the
>> changes themselves.
>
> Let me show it for you:
>
> bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
> {
> u8 dprx = 0;
>
> if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
> &dprx) != 1)
> return false;
> return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
> }
>
Even AMD has similar logic:
6145 stream->use_vsc_sdp_for_colorimetry = false;
6146 if (aconnector->dc_sink->sink_signal ==
SIGNAL_TYPE_DISPLAY_PORT_MST) {
6147 stream->use_vsc_sdp_for_colorimetry =
6148 aconnector->dc_sink->is_vsc_sdp_colorimetry_supported;
6149 } else {
6150 if
(stream->link->dpcd_caps.dprx_feature.bits.VSC_SDP_COLORIMETRY_SUPPORTED)
6151 stream->use_vsc_sdp_for_colorimetry = true;
6152 }
But it will be harder to untangle this compared to intel's code.
I am fine with adding an API to drm_dp_helper which indicates presence
of VSC SDP but I would prefer leaving it to other vendors to use it in
the way they would like and only keep msm usage in this series.
>
>>>
>>>>
>>>> Signed-off-by: Paloma Arellano <quic_parellan at quicinc.com>
>>>> ---
>>>> drivers/gpu/drm/msm/dp/dp_display.c | 3 ++-
>>>> drivers/gpu/drm/msm/dp/dp_panel.c | 35 +++++++++++++++++++++++++----
>>>> drivers/gpu/drm/msm/dp/dp_panel.h | 1 +
>>>> 3 files changed, 34 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
>>>> b/drivers/gpu/drm/msm/dp/dp_display.c
>>>> index ddac55f45a722..f6b3b6ca242f8 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>>> @@ -1617,7 +1617,8 @@ void dp_bridge_mode_set(struct drm_bridge
>>>> *drm_bridge,
>>>> !!(dp_display->dp_mode.drm_mode.flags & DRM_MODE_FLAG_NHSYNC);
>>>> dp_display->dp_mode.out_fmt_is_yuv_420 =
>>>> - drm_mode_is_420_only(&dp->connector->display_info, adjusted_mode);
>>>> + drm_mode_is_420_only(&dp->connector->display_info, adjusted_mode) &&
>>>> + dp_panel_vsc_sdp_supported(dp_display->panel);
>>>> /* populate wide_bus_support to different layers */
>>>> dp_display->ctrl->wide_bus_en =
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
>>>> b/drivers/gpu/drm/msm/dp/dp_panel.c
>>>> index 127f6af995cd1..af7820b6d35ec 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
>>>> @@ -17,6 +17,9 @@ struct dp_panel_private {
>>>> struct dp_link *link;
>>>> struct dp_catalog *catalog;
>>>> bool panel_on;
>>>> + bool vsc_supported;
>>>> + u8 major;
>>>> + u8 minor;
>>>> };
>>>> static void dp_panel_read_psr_cap(struct dp_panel_private *panel)
>>>> @@ -43,9 +46,10 @@ static void dp_panel_read_psr_cap(struct
>>>> dp_panel_private *panel)
>>>> static int dp_panel_read_dpcd(struct dp_panel *dp_panel)
>>>> {
>>>> int rc;
>>>> + ssize_t rlen;
>>>> struct dp_panel_private *panel;
>>>> struct dp_link_info *link_info;
>>>> - u8 *dpcd, major, minor;
>>>> + u8 *dpcd, rx_feature;
>>>> panel = container_of(dp_panel, struct dp_panel_private,
>>>> dp_panel);
>>>> dpcd = dp_panel->dpcd;
>>>> @@ -53,10 +57,19 @@ static int dp_panel_read_dpcd(struct dp_panel
>>>> *dp_panel)
>>>> if (rc)
>>>> return rc;
>>>> + rlen = drm_dp_dpcd_read(panel->aux,
>>>> DP_DPRX_FEATURE_ENUMERATION_LIST, &rx_feature, 1);
>>>> + if (rlen != 1) {
>>>> + panel->vsc_supported = false;
>>>> + pr_debug("failed to read DP_DPRX_FEATURE_ENUMERATION_LIST\n");
>>>> + } else {
>>>> + panel->vsc_supported = !!(rx_feature &
>>>> DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED);
>>>> + pr_debug("vsc=%d\n", panel->vsc_supported);
>>>> + }
>>>> +
>>>> link_info = &dp_panel->link_info;
>>>> link_info->revision = dpcd[DP_DPCD_REV];
>>>> - major = (link_info->revision >> 4) & 0x0f;
>>>> - minor = link_info->revision & 0x0f;
>>>> + panel->major = (link_info->revision >> 4) & 0x0f;
>>>> + panel->minor = link_info->revision & 0x0f;
>>>> link_info->rate = drm_dp_max_link_rate(dpcd);
>>>> link_info->num_lanes = drm_dp_max_lane_count(dpcd);
>>>> @@ -69,7 +82,7 @@ static int dp_panel_read_dpcd(struct dp_panel
>>>> *dp_panel)
>>>> if (link_info->rate > dp_panel->max_dp_link_rate)
>>>> link_info->rate = dp_panel->max_dp_link_rate;
>>>> - drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", major, minor);
>>>> + drm_dbg_dp(panel->drm_dev, "version: %d.%d\n", panel->major,
>>>> panel->minor);
>>>> drm_dbg_dp(panel->drm_dev, "link_rate=%d\n", link_info->rate);
>>>> drm_dbg_dp(panel->drm_dev, "lane_count=%d\n",
>>>> link_info->num_lanes);
>>>> @@ -280,6 +293,20 @@ void dp_panel_tpg_config(struct dp_panel
>>>> *dp_panel, bool enable)
>>>> dp_catalog_panel_tpg_enable(catalog,
>>>> &panel->dp_panel.dp_mode.drm_mode);
>>>> }
>>>> +bool dp_panel_vsc_sdp_supported(struct dp_panel *dp_panel)
>>>> +{
>>>> + struct dp_panel_private *panel;
>>>> +
>>>> + if (!dp_panel) {
>>>> + pr_err("invalid input\n");
>>>> + return false;
>>>> + }
>>>> +
>>>> + panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
>>>> +
>>>> + return panel->major >= 1 && panel->minor >= 3 &&
>>>> panel->vsc_supported;
>
> Anyway, this check is incorrect. Please compare the whole revision
> against DP_DPCD_REV_13 instead of doing a maj/min comparison.
>
>>>> +}
>>>> +
>>>> void dp_panel_dump_regs(struct dp_panel *dp_panel)
>>>> {
>>>> struct dp_catalog *catalog;
>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
>>>> b/drivers/gpu/drm/msm/dp/dp_panel.h
>>>> index 6ec68be9f2366..590eca5ce304b 100644
>>>> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
>>>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
>>>> @@ -66,6 +66,7 @@ int dp_panel_get_modes(struct dp_panel *dp_panel,
>>>> struct drm_connector *connector);
>>>> void dp_panel_handle_sink_request(struct dp_panel *dp_panel);
>>>> void dp_panel_tpg_config(struct dp_panel *dp_panel, bool enable);
>>>> +bool dp_panel_vsc_sdp_supported(struct dp_panel *dp_panel);
>>>> /**
>>>> * is_link_rate_valid() - validates the link rate
>>>
>
>
>
More information about the dri-devel
mailing list