Re: [PATCH 4/4] drm/edid: Avoid multiple log lines for HFVSDB parsing
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 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
---
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 = >display_info;
struct drm_hdmi_info *hdmi = >hdmi;
struct drm_hdmi_dsc_cap *hdmi_dsc = >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 = >scdc;
+ /* max clock is 5000 KHz times block value */
+ max_tmds_clock = hf_scds[5] * 5000;
+
if (max_tmds_clock > 34) {
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, >max_lanes,
>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,
Re: [PATCH 4/4] drm/edid: Avoid multiple log lines for HFVSDB parsing
On Thu, 11 Aug 2022, Ankit Nautiyal 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
> ---
> 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 = >display_info;
> struct drm_hdmi_info *hdmi = >hdmi;
> struct drm_hdmi_dsc_cap *hdmi_dsc = >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.
> + 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 = >scdc;
>
> + /* max clock is 5000 KHz times block value */
> + max_tmds_clock = hf_scds[5] * 5000;
> +
> if (max_tmds_clock > 34) {
> 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.
> }
>
> 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, >max_lanes,
>>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 ":".
> + max_tmds_clock, max_frl_rate ? "yes" : "no", dsc_support ?
> "yes" : "no");
See str_yes_no().
BR,
Jani.
> }
>
> static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
--
Jani Nikula, Intel Open Source Graphics Center
[PATCH 4/4] drm/edid: Avoid multiple log lines for HFVSDB parsing
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
---
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 = >display_info;
struct drm_hdmi_info *hdmi = >hdmi;
struct drm_hdmi_dsc_cap *hdmi_dsc = >dsc_cap;
+ u32 max_tmds_clock = 0;
+ 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 = >scdc;
+ /* max clock is 5000 KHz times block value */
+ max_tmds_clock = hf_scds[5] * 5000;
+
if (max_tmds_clock > 34) {
display->max_tmds_clock = max_tmds_clock;
- DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n",
- display->max_tmds_clock);
}
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, >max_lanes,
>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",
+ max_tmds_clock, max_frl_rate ? "yes" : "no", dsc_support ?
"yes" : "no");
}
static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
--
2.25.1
