[PATCH 4/4] drm/edid: Avoid multiple log lines for HFVSDB parsing
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Wed Sep 14 10:09:50 UTC 2022
Thanks Jani for the review and suggestions.
I agree with the suggestions and will make changes in next version.
Please find my response inline:
On 9/13/2022 7:24 PM, Jani Nikula wrote:
> On Thu, 11 Aug 2022, Ankit Nautiyal <ankit.k.nautiyal at intel.com> wrote:
>> Replace multiple log lines with a single log line at the end of
>> parsing HF-VSDB. Also use drm_dbg_kms instead of DRM_DBG_KMS, and
>> add log for DSC1.2 support.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> ---
>> drivers/gpu/drm/drm_edid.c | 21 +++++++++++++--------
>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index c9c3a9c8fa26..7a319d570297 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -5781,6 +5781,9 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector,
>> struct drm_display_info *display = &connector->display_info;
>> struct drm_hdmi_info *hdmi = &display->hdmi;
>> struct drm_hdmi_dsc_cap *hdmi_dsc = &hdmi->dsc_cap;
>> + u32 max_tmds_clock = 0;
> This should be int because display->max_tmds_clock is int. Yes, it's a
> change from the current local var, but logging u32 would require %u
> instead of %d in the format string anyway, so better just use the right
> type.
Alright, makes sense.
>> + u8 max_frl_rate = 0;
>> + bool dsc_support = false;
>>
>> display->has_hdmi_infoframe = true;
>>
>> @@ -5800,14 +5803,13 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector,
>> */
>>
>> if (hf_scds[5]) {
>> - /* max clock is 5000 KHz times block value */
>> - u32 max_tmds_clock = hf_scds[5] * 5000;
>> struct drm_scdc *scdc = &hdmi->scdc;
>>
>> + /* max clock is 5000 KHz times block value */
>> + max_tmds_clock = hf_scds[5] * 5000;
>> +
>> if (max_tmds_clock > 340000) {
>> display->max_tmds_clock = max_tmds_clock;
>> - DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
>> - display->max_tmds_clock);
> Hmm, the logic for what is logged gets changed.
You are right, we are now logging this always.
Should we log this only for rate > 340MHz? The logging line at last will
require some jugglery.
>> }
>>
>> if (scdc->supported) {
>> @@ -5820,9 +5822,6 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector,
>> }
>>
>> if (hf_scds[7]) {
>> - u8 max_frl_rate;
>> -
>> - DRM_DEBUG_KMS("hdmi_21 sink detected. parsing edid\n");
>> max_frl_rate = (hf_scds[7] & DRM_EDID_MAX_FRL_RATE_MASK) >> 4;
>> drm_get_max_frl_rate(max_frl_rate, &hdmi->max_lanes,
>> &hdmi->max_frl_rate_per_lane);
>> @@ -5830,8 +5829,14 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector,
>>
>> drm_parse_ycbcr420_deep_color_info(connector, hf_scds);
>>
>> - if (cea_db_payload_len(hf_scds) >= 11 && hf_scds[11])
>> + if (cea_db_payload_len(hf_scds) >= 11 && hf_scds[11]) {
>> drm_parse_dsc_info(hdmi_dsc, hf_scds);
>> + dsc_support = true;
>> + }
>> +
>> + drm_dbg_kms(connector->dev,
>> + "HF-VSDB: max TMDS clock:%d Khz, HDMI2.1 support:%s, DSC1.2 support:%s\n",
> Nitpicks, %d needs int instead of u32, "kHz" not "Khz", "HDMI 2.1" and
> "DSC 1.2" with spaces, would prefer a space after ":".
Noted, Will fix this.
>
>> + max_tmds_clock, max_frl_rate ? "yes" : "no", dsc_support ? "yes" : "no");
> See str_yes_no().
Right, should have used str_yes_no().
Thanks & Regards,
Ankit
>
> BR,
> Jani.
>
>> }
>>
>> static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
More information about the dri-devel
mailing list