| Message ID | 20241011152929.10145-1-vamsikrishna.brahmajosyula@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/edid: transition to passing struct cea_db * to cae_db_payload_len | expand |
On Fri, Oct 11, 2024 at 8:59 PM Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> wrote: > > Address the FIXME in cea_db_payload_len > Transition to passing struct cea_db * everywhere > > Precompute the payload length in drm_parse_cea_ext and pass to > individual parsers to avoid casting struct cea_db pointer to u8 > pointer where length is needed. > > Since the type of payload length is inconsistent in the file, > use u8, u16 where it was already in place, use int elsewhere. > > Signed-off-by: Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> > --- > drivers/gpu/drm/drm_edid.c | 63 ++++++++++++++++---------------------- > 1 file changed, 27 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 855beafb76ff..80442e8b8ac6 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4977,11 +4977,8 @@ static int cea_db_tag(const struct cea_db *db) > return db->tag_length >> 5; > } > > -static int cea_db_payload_len(const void *_db) > +static int cea_db_payload_len(const struct cea_db *db) > { > - /* FIXME: Transition to passing struct cea_db * everywhere. */ > - const struct cea_db *db = _db; > - > return db->tag_length & 0x1f; > } > > @@ -5432,22 +5429,18 @@ static uint8_t hdr_metadata_type(const u8 *edid_ext) > } > > static void > -drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) > +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db, const u16 payload_len) > { > - u16 len; > - > - len = cea_db_payload_len(db); > - > connector->hdr_sink_metadata.hdmi_type1.eotf = > eotf_supported(db); > connector->hdr_sink_metadata.hdmi_type1.metadata_type = > hdr_metadata_type(db); > > - if (len >= 4) > + if (payload_len >= 4) > connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; > - if (len >= 5) > + if (payload_len >= 5) > connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; > - if (len >= 6) { > + if (payload_len >= 6) { > connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; > > /* Calculate only when all values are available */ > @@ -5457,20 +5450,18 @@ drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) > > /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ > static void > -drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) > +drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db, u8 payload_len) > { > - u8 len = cea_db_payload_len(db); > - > - if (len >= 6 && (db[6] & (1 << 7))) > + if (payload_len >= 6 && (db[6] & (1 << 7))) > connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= DRM_ELD_SUPPORTS_AI; > > - if (len >= 10 && hdmi_vsdb_latency_present(db)) { > + if (payload_len >= 10 && hdmi_vsdb_latency_present(db)) { > connector->latency_present[0] = true; > connector->video_latency[0] = db[9]; > connector->audio_latency[0] = db[10]; > } > > - if (len >= 12 && hdmi_vsdb_i_latency_present(db)) { > + if (payload_len >= 12 && hdmi_vsdb_i_latency_present(db)) { > connector->latency_present[1] = true; > connector->video_latency[1] = db[11]; > connector->audio_latency[1] = db[12]; > @@ -5695,7 +5686,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, > case CTA_DB_VENDOR: > /* HDMI Vendor-Specific Data Block */ > if (cea_db_is_hdmi_vsdb(db)) > - drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db); > + drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db, len); > break; > default: > break; > @@ -6122,7 +6113,7 @@ static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector, > } > > static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, > - const u8 *hf_scds) > + const u8 *hf_scds, int payload_len) > { > hdmi_dsc->v_1p2 = hf_scds[11] & DRM_EDID_DSC_1P2; > > @@ -6142,7 +6133,7 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, > /* Supports min 8 BPC if DSC 1.2 is supported*/ > hdmi_dsc->bpc_supported = 8; > > - if (cea_db_payload_len(hf_scds) >= 12 && hf_scds[12]) { > + if (payload_len >= 12 && hf_scds[12]) { > u8 dsc_max_slices; > u8 dsc_max_frl_rate; > > @@ -6188,13 +6179,13 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, > } > } > > - if (cea_db_payload_len(hf_scds) >= 13 && hf_scds[13]) > + if (payload_len >= 13 && hf_scds[13]) > hdmi_dsc->total_chunk_kbytes = hf_scds[13] & DRM_EDID_DSC_TOTAL_CHUNK_KBYTES; > } > > /* Sink Capability Data Structure */ > static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, > - const u8 *hf_scds) > + const u8 *hf_scds, int payload_len) > { > struct drm_display_info *info = &connector->display_info; > struct drm_hdmi_info *hdmi = &info->hdmi; > @@ -6247,8 +6238,8 @@ 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]) { > - drm_parse_dsc_info(hdmi_dsc, hf_scds); > + if (payload_len >= 11 && hf_scds[11]) { > + drm_parse_dsc_info(hdmi_dsc, hf_scds, payload_len); > dsc_support = true; > } > > @@ -6259,7 +6250,7 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, > } > > static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, > - const u8 *hdmi) > + const u8 *hdmi, const u8 payload_len) > { > struct drm_display_info *info = &connector->display_info; > unsigned int dc_bpc = 0; > @@ -6267,7 +6258,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, > /* HDMI supports at least 8 bpc */ > info->bpc = 8; > > - if (cea_db_payload_len(hdmi) < 6) > + if (payload_len < 6) > return; > > if (hdmi[6] & DRM_EDID_HDMI_DC_30) { > @@ -6320,18 +6311,17 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, > > /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ > static void > -drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) > +drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db, const u8 payload_len) > { > struct drm_display_info *info = &connector->display_info; > - u8 len = cea_db_payload_len(db); > > info->is_hdmi = true; > > info->source_physical_address = (db[4] << 8) | db[5]; > > - if (len >= 6) > + if (payload_len >= 6) > info->dvi_dual = db[6] & 1; > - if (len >= 7) > + if (payload_len >= 7) > info->max_tmds_clock = db[7] * 5000; > > /* > @@ -6340,14 +6330,14 @@ drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) > * HDMI infoframe support was first added in HDMI 1.4. Assume the sink > * supports infoframes if HDMI_Video_present is set. > */ > - if (len >= 8 && db[8] & BIT(5)) > + if (payload_len >= 8 && db[8] & BIT(5)) > info->has_hdmi_infoframe = true; > > drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI: DVI dual %d, max TMDS clock %d kHz\n", > connector->base.id, connector->name, > info->dvi_dual, info->max_tmds_clock); > > - drm_parse_hdmi_deep_color_info(connector, db); > + drm_parse_hdmi_deep_color_info(connector, db, payload_len); > } > > /* > @@ -6410,12 +6400,13 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > cea_db_iter_for_each(db, &iter) { > /* FIXME: convert parsers to use struct cea_db */ > const u8 *data = (const u8 *)db; > + int len = cea_db_payload_len(db); > > if (cea_db_is_hdmi_vsdb(db)) > - drm_parse_hdmi_vsdb_video(connector, data); > + drm_parse_hdmi_vsdb_video(connector, data, len); > else if (cea_db_is_hdmi_forum_vsdb(db) || > cea_db_is_hdmi_forum_scdb(db)) > - drm_parse_hdmi_forum_scds(connector, data); > + drm_parse_hdmi_forum_scds(connector, data, len); > else if (cea_db_is_microsoft_vsdb(db)) > drm_parse_microsoft_vsdb(connector, data); > else if (cea_db_is_y420cmdb(db)) > @@ -6425,7 +6416,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > else if (cea_db_is_vcdb(db)) > drm_parse_vcdb(connector, data); > else if (cea_db_is_hdmi_hdr_metadata_block(db)) > - drm_parse_hdr_metadata_block(connector, data); > + drm_parse_hdr_metadata_block(connector, data, len); > else if (cea_db_tag(db) == CTA_DB_VIDEO) > parse_cta_vdb(connector, db); > else if (cea_db_tag(db) == CTA_DB_AUDIO) > > base-commit: 1d227fcc72223cbdd34d0ce13541cbaab5e0d72f > -- > 2.47.0 > gentle reminder.. Would love to hear any feedback on the patch. Thanks, Vamsi
On Fri, 11 Oct 2024, Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> wrote: > Address the FIXME in cea_db_payload_len > Transition to passing struct cea_db * everywhere You've misunderstood the comment. The point is to pass struct cea_db * around, not to stick to u8 * and drop calls to cea_db_payload_len(). BR, Jani. > > Precompute the payload length in drm_parse_cea_ext and pass to > individual parsers to avoid casting struct cea_db pointer to u8 > pointer where length is needed. > > Since the type of payload length is inconsistent in the file, > use u8, u16 where it was already in place, use int elsewhere. > > Signed-off-by: Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> > --- > drivers/gpu/drm/drm_edid.c | 63 ++++++++++++++++---------------------- > 1 file changed, 27 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 855beafb76ff..80442e8b8ac6 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4977,11 +4977,8 @@ static int cea_db_tag(const struct cea_db *db) > return db->tag_length >> 5; > } > > -static int cea_db_payload_len(const void *_db) > +static int cea_db_payload_len(const struct cea_db *db) > { > - /* FIXME: Transition to passing struct cea_db * everywhere. */ > - const struct cea_db *db = _db; > - > return db->tag_length & 0x1f; > } > > @@ -5432,22 +5429,18 @@ static uint8_t hdr_metadata_type(const u8 *edid_ext) > } > > static void > -drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) > +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db, const u16 payload_len) > { > - u16 len; > - > - len = cea_db_payload_len(db); > - > connector->hdr_sink_metadata.hdmi_type1.eotf = > eotf_supported(db); > connector->hdr_sink_metadata.hdmi_type1.metadata_type = > hdr_metadata_type(db); > > - if (len >= 4) > + if (payload_len >= 4) > connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; > - if (len >= 5) > + if (payload_len >= 5) > connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; > - if (len >= 6) { > + if (payload_len >= 6) { > connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; > > /* Calculate only when all values are available */ > @@ -5457,20 +5450,18 @@ drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) > > /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ > static void > -drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) > +drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db, u8 payload_len) > { > - u8 len = cea_db_payload_len(db); > - > - if (len >= 6 && (db[6] & (1 << 7))) > + if (payload_len >= 6 && (db[6] & (1 << 7))) > connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= DRM_ELD_SUPPORTS_AI; > > - if (len >= 10 && hdmi_vsdb_latency_present(db)) { > + if (payload_len >= 10 && hdmi_vsdb_latency_present(db)) { > connector->latency_present[0] = true; > connector->video_latency[0] = db[9]; > connector->audio_latency[0] = db[10]; > } > > - if (len >= 12 && hdmi_vsdb_i_latency_present(db)) { > + if (payload_len >= 12 && hdmi_vsdb_i_latency_present(db)) { > connector->latency_present[1] = true; > connector->video_latency[1] = db[11]; > connector->audio_latency[1] = db[12]; > @@ -5695,7 +5686,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, > case CTA_DB_VENDOR: > /* HDMI Vendor-Specific Data Block */ > if (cea_db_is_hdmi_vsdb(db)) > - drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db); > + drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db, len); > break; > default: > break; > @@ -6122,7 +6113,7 @@ static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector, > } > > static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, > - const u8 *hf_scds) > + const u8 *hf_scds, int payload_len) > { > hdmi_dsc->v_1p2 = hf_scds[11] & DRM_EDID_DSC_1P2; > > @@ -6142,7 +6133,7 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, > /* Supports min 8 BPC if DSC 1.2 is supported*/ > hdmi_dsc->bpc_supported = 8; > > - if (cea_db_payload_len(hf_scds) >= 12 && hf_scds[12]) { > + if (payload_len >= 12 && hf_scds[12]) { > u8 dsc_max_slices; > u8 dsc_max_frl_rate; > > @@ -6188,13 +6179,13 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, > } > } > > - if (cea_db_payload_len(hf_scds) >= 13 && hf_scds[13]) > + if (payload_len >= 13 && hf_scds[13]) > hdmi_dsc->total_chunk_kbytes = hf_scds[13] & DRM_EDID_DSC_TOTAL_CHUNK_KBYTES; > } > > /* Sink Capability Data Structure */ > static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, > - const u8 *hf_scds) > + const u8 *hf_scds, int payload_len) > { > struct drm_display_info *info = &connector->display_info; > struct drm_hdmi_info *hdmi = &info->hdmi; > @@ -6247,8 +6238,8 @@ 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]) { > - drm_parse_dsc_info(hdmi_dsc, hf_scds); > + if (payload_len >= 11 && hf_scds[11]) { > + drm_parse_dsc_info(hdmi_dsc, hf_scds, payload_len); > dsc_support = true; > } > > @@ -6259,7 +6250,7 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, > } > > static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, > - const u8 *hdmi) > + const u8 *hdmi, const u8 payload_len) > { > struct drm_display_info *info = &connector->display_info; > unsigned int dc_bpc = 0; > @@ -6267,7 +6258,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, > /* HDMI supports at least 8 bpc */ > info->bpc = 8; > > - if (cea_db_payload_len(hdmi) < 6) > + if (payload_len < 6) > return; > > if (hdmi[6] & DRM_EDID_HDMI_DC_30) { > @@ -6320,18 +6311,17 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, > > /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ > static void > -drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) > +drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db, const u8 payload_len) > { > struct drm_display_info *info = &connector->display_info; > - u8 len = cea_db_payload_len(db); > > info->is_hdmi = true; > > info->source_physical_address = (db[4] << 8) | db[5]; > > - if (len >= 6) > + if (payload_len >= 6) > info->dvi_dual = db[6] & 1; > - if (len >= 7) > + if (payload_len >= 7) > info->max_tmds_clock = db[7] * 5000; > > /* > @@ -6340,14 +6330,14 @@ drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) > * HDMI infoframe support was first added in HDMI 1.4. Assume the sink > * supports infoframes if HDMI_Video_present is set. > */ > - if (len >= 8 && db[8] & BIT(5)) > + if (payload_len >= 8 && db[8] & BIT(5)) > info->has_hdmi_infoframe = true; > > drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI: DVI dual %d, max TMDS clock %d kHz\n", > connector->base.id, connector->name, > info->dvi_dual, info->max_tmds_clock); > > - drm_parse_hdmi_deep_color_info(connector, db); > + drm_parse_hdmi_deep_color_info(connector, db, payload_len); > } > > /* > @@ -6410,12 +6400,13 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > cea_db_iter_for_each(db, &iter) { > /* FIXME: convert parsers to use struct cea_db */ > const u8 *data = (const u8 *)db; > + int len = cea_db_payload_len(db); > > if (cea_db_is_hdmi_vsdb(db)) > - drm_parse_hdmi_vsdb_video(connector, data); > + drm_parse_hdmi_vsdb_video(connector, data, len); > else if (cea_db_is_hdmi_forum_vsdb(db) || > cea_db_is_hdmi_forum_scdb(db)) > - drm_parse_hdmi_forum_scds(connector, data); > + drm_parse_hdmi_forum_scds(connector, data, len); > else if (cea_db_is_microsoft_vsdb(db)) > drm_parse_microsoft_vsdb(connector, data); > else if (cea_db_is_y420cmdb(db)) > @@ -6425,7 +6416,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > else if (cea_db_is_vcdb(db)) > drm_parse_vcdb(connector, data); > else if (cea_db_is_hdmi_hdr_metadata_block(db)) > - drm_parse_hdr_metadata_block(connector, data); > + drm_parse_hdr_metadata_block(connector, data, len); > else if (cea_db_tag(db) == CTA_DB_VIDEO) > parse_cta_vdb(connector, db); > else if (cea_db_tag(db) == CTA_DB_AUDIO) > > base-commit: 1d227fcc72223cbdd34d0ce13541cbaab5e0d72f
On Mon, Oct 21, 2024 at 7:12 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 11 Oct 2024, Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> wrote: > > Address the FIXME in cea_db_payload_len > > Transition to passing struct cea_db * everywhere > > You've misunderstood the comment. The point is to pass struct cea_db * > around, not to stick to u8 * and drop calls to cea_db_payload_len(). > Thank you for the response. Would below be an acceptable change for the FIXME? 1. Use a macro to extract length from (db->tag_length & 0x1f) 2. Pass struct cea_db * to all individual parsers 3. Use db->data? / convert to u8 and use db[i] where needed. > BR, > Jani. > > > > > Precompute the payload length in drm_parse_cea_ext and pass to > > individual parsers to avoid casting struct cea_db pointer to u8 > > pointer where length is needed. > > > > Since the type of payload length is inconsistent in the file, > > use u8, u16 where it was already in place, use int elsewhere. > > > > Signed-off-by: Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> > > --- > > drivers/gpu/drm/drm_edid.c | 63 ++++++++++++++++---------------------- > > 1 file changed, 27 insertions(+), 36 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 855beafb76ff..80442e8b8ac6 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -4977,11 +4977,8 @@ static int cea_db_tag(const struct cea_db *db) > > return db->tag_length >> 5; > > } > > > > -static int cea_db_payload_len(const void *_db) > > +static int cea_db_payload_len(const struct cea_db *db) > > { > > - /* FIXME: Transition to passing struct cea_db * everywhere. */ > > - const struct cea_db *db = _db; > > - > > return db->tag_length & 0x1f; > > } > > > > @@ -5432,22 +5429,18 @@ static uint8_t hdr_metadata_type(const u8 *edid_ext) > > } > > > > static void > > -drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) > > +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db, const u16 payload_len) > > { > > - u16 len; > > - > > - len = cea_db_payload_len(db); > > - > > connector->hdr_sink_metadata.hdmi_type1.eotf = > > eotf_supported(db); > > connector->hdr_sink_metadata.hdmi_type1.metadata_type = > > hdr_metadata_type(db); > > > > - if (len >= 4) > > + if (payload_len >= 4) > > connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; > > - if (len >= 5) > > + if (payload_len >= 5) > > connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; > > - if (len >= 6) { > > + if (payload_len >= 6) { > > connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; > > > > /* Calculate only when all values are available */ > > @@ -5457,20 +5450,18 @@ drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) > > > > /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ > > static void > > -drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) > > +drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db, u8 payload_len) > > { > > - u8 len = cea_db_payload_len(db); > > - > > - if (len >= 6 && (db[6] & (1 << 7))) > > + if (payload_len >= 6 && (db[6] & (1 << 7))) > > connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= DRM_ELD_SUPPORTS_AI; > > > > - if (len >= 10 && hdmi_vsdb_latency_present(db)) { > > + if (payload_len >= 10 && hdmi_vsdb_latency_present(db)) { > > connector->latency_present[0] = true; > > connector->video_latency[0] = db[9]; > > connector->audio_latency[0] = db[10]; > > } > > > > - if (len >= 12 && hdmi_vsdb_i_latency_present(db)) { > > + if (payload_len >= 12 && hdmi_vsdb_i_latency_present(db)) { > > connector->latency_present[1] = true; > > connector->video_latency[1] = db[11]; > > connector->audio_latency[1] = db[12]; > > @@ -5695,7 +5686,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, > > case CTA_DB_VENDOR: > > /* HDMI Vendor-Specific Data Block */ > > if (cea_db_is_hdmi_vsdb(db)) > > - drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db); > > + drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db, len); > > break; > > default: > > break; > > @@ -6122,7 +6113,7 @@ static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector, > > } > > > > static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, > > - const u8 *hf_scds) > > + const u8 *hf_scds, int payload_len) > > { > > hdmi_dsc->v_1p2 = hf_scds[11] & DRM_EDID_DSC_1P2; > > > > @@ -6142,7 +6133,7 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, > > /* Supports min 8 BPC if DSC 1.2 is supported*/ > > hdmi_dsc->bpc_supported = 8; > > > > - if (cea_db_payload_len(hf_scds) >= 12 && hf_scds[12]) { > > + if (payload_len >= 12 && hf_scds[12]) { > > u8 dsc_max_slices; > > u8 dsc_max_frl_rate; > > > > @@ -6188,13 +6179,13 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, > > } > > } > > > > - if (cea_db_payload_len(hf_scds) >= 13 && hf_scds[13]) > > + if (payload_len >= 13 && hf_scds[13]) > > hdmi_dsc->total_chunk_kbytes = hf_scds[13] & DRM_EDID_DSC_TOTAL_CHUNK_KBYTES; > > } > > > > /* Sink Capability Data Structure */ > > static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, > > - const u8 *hf_scds) > > + const u8 *hf_scds, int payload_len) > > { > > struct drm_display_info *info = &connector->display_info; > > struct drm_hdmi_info *hdmi = &info->hdmi; > > @@ -6247,8 +6238,8 @@ 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]) { > > - drm_parse_dsc_info(hdmi_dsc, hf_scds); > > + if (payload_len >= 11 && hf_scds[11]) { > > + drm_parse_dsc_info(hdmi_dsc, hf_scds, payload_len); > > dsc_support = true; > > } > > > > @@ -6259,7 +6250,7 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, > > } > > > > static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, > > - const u8 *hdmi) > > + const u8 *hdmi, const u8 payload_len) > > { > > struct drm_display_info *info = &connector->display_info; > > unsigned int dc_bpc = 0; > > @@ -6267,7 +6258,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, > > /* HDMI supports at least 8 bpc */ > > info->bpc = 8; > > > > - if (cea_db_payload_len(hdmi) < 6) > > + if (payload_len < 6) > > return; > > > > if (hdmi[6] & DRM_EDID_HDMI_DC_30) { > > @@ -6320,18 +6311,17 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, > > > > /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ > > static void > > -drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) > > +drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db, const u8 payload_len) > > { > > struct drm_display_info *info = &connector->display_info; > > - u8 len = cea_db_payload_len(db); > > > > info->is_hdmi = true; > > > > info->source_physical_address = (db[4] << 8) | db[5]; > > > > - if (len >= 6) > > + if (payload_len >= 6) > > info->dvi_dual = db[6] & 1; > > - if (len >= 7) > > + if (payload_len >= 7) > > info->max_tmds_clock = db[7] * 5000; > > > > /* > > @@ -6340,14 +6330,14 @@ drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) > > * HDMI infoframe support was first added in HDMI 1.4. Assume the sink > > * supports infoframes if HDMI_Video_present is set. > > */ > > - if (len >= 8 && db[8] & BIT(5)) > > + if (payload_len >= 8 && db[8] & BIT(5)) > > info->has_hdmi_infoframe = true; > > > > drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI: DVI dual %d, max TMDS clock %d kHz\n", > > connector->base.id, connector->name, > > info->dvi_dual, info->max_tmds_clock); > > > > - drm_parse_hdmi_deep_color_info(connector, db); > > + drm_parse_hdmi_deep_color_info(connector, db, payload_len); > > } > > > > /* > > @@ -6410,12 +6400,13 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > > cea_db_iter_for_each(db, &iter) { > > /* FIXME: convert parsers to use struct cea_db */ > > const u8 *data = (const u8 *)db; > > + int len = cea_db_payload_len(db); > > > > if (cea_db_is_hdmi_vsdb(db)) > > - drm_parse_hdmi_vsdb_video(connector, data); > > + drm_parse_hdmi_vsdb_video(connector, data, len); > > else if (cea_db_is_hdmi_forum_vsdb(db) || > > cea_db_is_hdmi_forum_scdb(db)) > > - drm_parse_hdmi_forum_scds(connector, data); > > + drm_parse_hdmi_forum_scds(connector, data, len); > > else if (cea_db_is_microsoft_vsdb(db)) > > drm_parse_microsoft_vsdb(connector, data); > > else if (cea_db_is_y420cmdb(db)) > > @@ -6425,7 +6416,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > > else if (cea_db_is_vcdb(db)) > > drm_parse_vcdb(connector, data); > > else if (cea_db_is_hdmi_hdr_metadata_block(db)) > > - drm_parse_hdr_metadata_block(connector, data); > > + drm_parse_hdr_metadata_block(connector, data, len); > > else if (cea_db_tag(db) == CTA_DB_VIDEO) > > parse_cta_vdb(connector, db); > > else if (cea_db_tag(db) == CTA_DB_AUDIO) > > > > base-commit: 1d227fcc72223cbdd34d0ce13541cbaab5e0d72f > > -- > Jani Nikula, Intel Thanks, Vamsi
On Mon, 21 Oct 2024, Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> wrote: > On Mon, Oct 21, 2024 at 7:12 PM Jani Nikula <jani.nikula@linux.intel.com> wrote: >> >> On Fri, 11 Oct 2024, Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> wrote: >> > Address the FIXME in cea_db_payload_len >> > Transition to passing struct cea_db * everywhere >> >> You've misunderstood the comment. The point is to pass struct cea_db * >> around, not to stick to u8 * and drop calls to cea_db_payload_len(). >> > Thank you for the response. > Would below be an acceptable change for the FIXME? > 1. Use a macro to extract length from (db->tag_length & 0x1f) No, why would you do that? We have the function. > 2. Pass struct cea_db * to all individual parsers Yes. Look at all callers of cea_db_payload_len() and propagate struct cea_db *db to them instead of u8 *db. The related and relevant other FIXME comment is in drm_parse_cea_ext(): /* FIXME: convert parsers to use struct cea_db */ That's where you should start. Some of the parsers already handle struct cea_db, and others need to use const u8 *data. Please don't cram all of this in one patch, probably start off function by function. It's easier to squash patches later than to split. Yes. > 3. Use db->data? / convert to u8 and use db[i] where needed. There's cea_db_data() for this. The slightly annoying part is that for extended tags and vendor blocks the "actual" data is at an offset. See for example drm_parse_hdmi_vsdb_video(). It's good that the len checks and db dereferences match, and that they match what's written in the spec, but it's a bit funny in the code. Maybe not something you need to worry about at this point. BR, Jani. > >> BR, >> Jani. >> >> > >> > Precompute the payload length in drm_parse_cea_ext and pass to >> > individual parsers to avoid casting struct cea_db pointer to u8 >> > pointer where length is needed. >> > >> > Since the type of payload length is inconsistent in the file, >> > use u8, u16 where it was already in place, use int elsewhere. >> > >> > Signed-off-by: Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> >> > --- >> > drivers/gpu/drm/drm_edid.c | 63 ++++++++++++++++---------------------- >> > 1 file changed, 27 insertions(+), 36 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> > index 855beafb76ff..80442e8b8ac6 100644 >> > --- a/drivers/gpu/drm/drm_edid.c >> > +++ b/drivers/gpu/drm/drm_edid.c >> > @@ -4977,11 +4977,8 @@ static int cea_db_tag(const struct cea_db *db) >> > return db->tag_length >> 5; >> > } >> > >> > -static int cea_db_payload_len(const void *_db) >> > +static int cea_db_payload_len(const struct cea_db *db) >> > { >> > - /* FIXME: Transition to passing struct cea_db * everywhere. */ >> > - const struct cea_db *db = _db; >> > - >> > return db->tag_length & 0x1f; >> > } >> > >> > @@ -5432,22 +5429,18 @@ static uint8_t hdr_metadata_type(const u8 *edid_ext) >> > } >> > >> > static void >> > -drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) >> > +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db, const u16 payload_len) >> > { >> > - u16 len; >> > - >> > - len = cea_db_payload_len(db); >> > - >> > connector->hdr_sink_metadata.hdmi_type1.eotf = >> > eotf_supported(db); >> > connector->hdr_sink_metadata.hdmi_type1.metadata_type = >> > hdr_metadata_type(db); >> > >> > - if (len >= 4) >> > + if (payload_len >= 4) >> > connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; >> > - if (len >= 5) >> > + if (payload_len >= 5) >> > connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; >> > - if (len >= 6) { >> > + if (payload_len >= 6) { >> > connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; >> > >> > /* Calculate only when all values are available */ >> > @@ -5457,20 +5450,18 @@ drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) >> > >> > /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ >> > static void >> > -drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) >> > +drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db, u8 payload_len) >> > { >> > - u8 len = cea_db_payload_len(db); >> > - >> > - if (len >= 6 && (db[6] & (1 << 7))) >> > + if (payload_len >= 6 && (db[6] & (1 << 7))) >> > connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= DRM_ELD_SUPPORTS_AI; >> > >> > - if (len >= 10 && hdmi_vsdb_latency_present(db)) { >> > + if (payload_len >= 10 && hdmi_vsdb_latency_present(db)) { >> > connector->latency_present[0] = true; >> > connector->video_latency[0] = db[9]; >> > connector->audio_latency[0] = db[10]; >> > } >> > >> > - if (len >= 12 && hdmi_vsdb_i_latency_present(db)) { >> > + if (payload_len >= 12 && hdmi_vsdb_i_latency_present(db)) { >> > connector->latency_present[1] = true; >> > connector->video_latency[1] = db[11]; >> > connector->audio_latency[1] = db[12]; >> > @@ -5695,7 +5686,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, >> > case CTA_DB_VENDOR: >> > /* HDMI Vendor-Specific Data Block */ >> > if (cea_db_is_hdmi_vsdb(db)) >> > - drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db); >> > + drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db, len); >> > break; >> > default: >> > break; >> > @@ -6122,7 +6113,7 @@ static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector, >> > } >> > >> > static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, >> > - const u8 *hf_scds) >> > + const u8 *hf_scds, int payload_len) >> > { >> > hdmi_dsc->v_1p2 = hf_scds[11] & DRM_EDID_DSC_1P2; >> > >> > @@ -6142,7 +6133,7 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, >> > /* Supports min 8 BPC if DSC 1.2 is supported*/ >> > hdmi_dsc->bpc_supported = 8; >> > >> > - if (cea_db_payload_len(hf_scds) >= 12 && hf_scds[12]) { >> > + if (payload_len >= 12 && hf_scds[12]) { >> > u8 dsc_max_slices; >> > u8 dsc_max_frl_rate; >> > >> > @@ -6188,13 +6179,13 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, >> > } >> > } >> > >> > - if (cea_db_payload_len(hf_scds) >= 13 && hf_scds[13]) >> > + if (payload_len >= 13 && hf_scds[13]) >> > hdmi_dsc->total_chunk_kbytes = hf_scds[13] & DRM_EDID_DSC_TOTAL_CHUNK_KBYTES; >> > } >> > >> > /* Sink Capability Data Structure */ >> > static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, >> > - const u8 *hf_scds) >> > + const u8 *hf_scds, int payload_len) >> > { >> > struct drm_display_info *info = &connector->display_info; >> > struct drm_hdmi_info *hdmi = &info->hdmi; >> > @@ -6247,8 +6238,8 @@ 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]) { >> > - drm_parse_dsc_info(hdmi_dsc, hf_scds); >> > + if (payload_len >= 11 && hf_scds[11]) { >> > + drm_parse_dsc_info(hdmi_dsc, hf_scds, payload_len); >> > dsc_support = true; >> > } >> > >> > @@ -6259,7 +6250,7 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, >> > } >> > >> > static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, >> > - const u8 *hdmi) >> > + const u8 *hdmi, const u8 payload_len) >> > { >> > struct drm_display_info *info = &connector->display_info; >> > unsigned int dc_bpc = 0; >> > @@ -6267,7 +6258,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, >> > /* HDMI supports at least 8 bpc */ >> > info->bpc = 8; >> > >> > - if (cea_db_payload_len(hdmi) < 6) >> > + if (payload_len < 6) >> > return; >> > >> > if (hdmi[6] & DRM_EDID_HDMI_DC_30) { >> > @@ -6320,18 +6311,17 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, >> > >> > /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ >> > static void >> > -drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) >> > +drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db, const u8 payload_len) >> > { >> > struct drm_display_info *info = &connector->display_info; >> > - u8 len = cea_db_payload_len(db); >> > >> > info->is_hdmi = true; >> > >> > info->source_physical_address = (db[4] << 8) | db[5]; >> > >> > - if (len >= 6) >> > + if (payload_len >= 6) >> > info->dvi_dual = db[6] & 1; >> > - if (len >= 7) >> > + if (payload_len >= 7) >> > info->max_tmds_clock = db[7] * 5000; >> > >> > /* >> > @@ -6340,14 +6330,14 @@ drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) >> > * HDMI infoframe support was first added in HDMI 1.4. Assume the sink >> > * supports infoframes if HDMI_Video_present is set. >> > */ >> > - if (len >= 8 && db[8] & BIT(5)) >> > + if (payload_len >= 8 && db[8] & BIT(5)) >> > info->has_hdmi_infoframe = true; >> > >> > drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI: DVI dual %d, max TMDS clock %d kHz\n", >> > connector->base.id, connector->name, >> > info->dvi_dual, info->max_tmds_clock); >> > >> > - drm_parse_hdmi_deep_color_info(connector, db); >> > + drm_parse_hdmi_deep_color_info(connector, db, payload_len); >> > } >> > >> > /* >> > @@ -6410,12 +6400,13 @@ static void drm_parse_cea_ext(struct drm_connector *connector, >> > cea_db_iter_for_each(db, &iter) { >> > /* FIXME: convert parsers to use struct cea_db */ >> > const u8 *data = (const u8 *)db; >> > + int len = cea_db_payload_len(db); >> > >> > if (cea_db_is_hdmi_vsdb(db)) >> > - drm_parse_hdmi_vsdb_video(connector, data); >> > + drm_parse_hdmi_vsdb_video(connector, data, len); >> > else if (cea_db_is_hdmi_forum_vsdb(db) || >> > cea_db_is_hdmi_forum_scdb(db)) >> > - drm_parse_hdmi_forum_scds(connector, data); >> > + drm_parse_hdmi_forum_scds(connector, data, len); >> > else if (cea_db_is_microsoft_vsdb(db)) >> > drm_parse_microsoft_vsdb(connector, data); >> > else if (cea_db_is_y420cmdb(db)) >> > @@ -6425,7 +6416,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector, >> > else if (cea_db_is_vcdb(db)) >> > drm_parse_vcdb(connector, data); >> > else if (cea_db_is_hdmi_hdr_metadata_block(db)) >> > - drm_parse_hdr_metadata_block(connector, data); >> > + drm_parse_hdr_metadata_block(connector, data, len); >> > else if (cea_db_tag(db) == CTA_DB_VIDEO) >> > parse_cta_vdb(connector, db); >> > else if (cea_db_tag(db) == CTA_DB_AUDIO) >> > >> > base-commit: 1d227fcc72223cbdd34d0ce13541cbaab5e0d72f >> >> -- >> Jani Nikula, Intel > Thanks, > Vamsi
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 855beafb76ff..80442e8b8ac6 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4977,11 +4977,8 @@ static int cea_db_tag(const struct cea_db *db) return db->tag_length >> 5; } -static int cea_db_payload_len(const void *_db) +static int cea_db_payload_len(const struct cea_db *db) { - /* FIXME: Transition to passing struct cea_db * everywhere. */ - const struct cea_db *db = _db; - return db->tag_length & 0x1f; } @@ -5432,22 +5429,18 @@ static uint8_t hdr_metadata_type(const u8 *edid_ext) } static void -drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) +drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db, const u16 payload_len) { - u16 len; - - len = cea_db_payload_len(db); - connector->hdr_sink_metadata.hdmi_type1.eotf = eotf_supported(db); connector->hdr_sink_metadata.hdmi_type1.metadata_type = hdr_metadata_type(db); - if (len >= 4) + if (payload_len >= 4) connector->hdr_sink_metadata.hdmi_type1.max_cll = db[4]; - if (len >= 5) + if (payload_len >= 5) connector->hdr_sink_metadata.hdmi_type1.max_fall = db[5]; - if (len >= 6) { + if (payload_len >= 6) { connector->hdr_sink_metadata.hdmi_type1.min_cll = db[6]; /* Calculate only when all values are available */ @@ -5457,20 +5450,18 @@ drm_parse_hdr_metadata_block(struct drm_connector *connector, const u8 *db) /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ static void -drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db) +drm_parse_hdmi_vsdb_audio(struct drm_connector *connector, const u8 *db, u8 payload_len) { - u8 len = cea_db_payload_len(db); - - if (len >= 6 && (db[6] & (1 << 7))) + if (payload_len >= 6 && (db[6] & (1 << 7))) connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= DRM_ELD_SUPPORTS_AI; - if (len >= 10 && hdmi_vsdb_latency_present(db)) { + if (payload_len >= 10 && hdmi_vsdb_latency_present(db)) { connector->latency_present[0] = true; connector->video_latency[0] = db[9]; connector->audio_latency[0] = db[10]; } - if (len >= 12 && hdmi_vsdb_i_latency_present(db)) { + if (payload_len >= 12 && hdmi_vsdb_i_latency_present(db)) { connector->latency_present[1] = true; connector->video_latency[1] = db[11]; connector->audio_latency[1] = db[12]; @@ -5695,7 +5686,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, case CTA_DB_VENDOR: /* HDMI Vendor-Specific Data Block */ if (cea_db_is_hdmi_vsdb(db)) - drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db); + drm_parse_hdmi_vsdb_audio(connector, (const u8 *)db, len); break; default: break; @@ -6122,7 +6113,7 @@ static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector, } static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, - const u8 *hf_scds) + const u8 *hf_scds, int payload_len) { hdmi_dsc->v_1p2 = hf_scds[11] & DRM_EDID_DSC_1P2; @@ -6142,7 +6133,7 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, /* Supports min 8 BPC if DSC 1.2 is supported*/ hdmi_dsc->bpc_supported = 8; - if (cea_db_payload_len(hf_scds) >= 12 && hf_scds[12]) { + if (payload_len >= 12 && hf_scds[12]) { u8 dsc_max_slices; u8 dsc_max_frl_rate; @@ -6188,13 +6179,13 @@ static void drm_parse_dsc_info(struct drm_hdmi_dsc_cap *hdmi_dsc, } } - if (cea_db_payload_len(hf_scds) >= 13 && hf_scds[13]) + if (payload_len >= 13 && hf_scds[13]) hdmi_dsc->total_chunk_kbytes = hf_scds[13] & DRM_EDID_DSC_TOTAL_CHUNK_KBYTES; } /* Sink Capability Data Structure */ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, - const u8 *hf_scds) + const u8 *hf_scds, int payload_len) { struct drm_display_info *info = &connector->display_info; struct drm_hdmi_info *hdmi = &info->hdmi; @@ -6247,8 +6238,8 @@ 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]) { - drm_parse_dsc_info(hdmi_dsc, hf_scds); + if (payload_len >= 11 && hf_scds[11]) { + drm_parse_dsc_info(hdmi_dsc, hf_scds, payload_len); dsc_support = true; } @@ -6259,7 +6250,7 @@ static void drm_parse_hdmi_forum_scds(struct drm_connector *connector, } static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, - const u8 *hdmi) + const u8 *hdmi, const u8 payload_len) { struct drm_display_info *info = &connector->display_info; unsigned int dc_bpc = 0; @@ -6267,7 +6258,7 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, /* HDMI supports at least 8 bpc */ info->bpc = 8; - if (cea_db_payload_len(hdmi) < 6) + if (payload_len < 6) return; if (hdmi[6] & DRM_EDID_HDMI_DC_30) { @@ -6320,18 +6311,17 @@ static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, /* HDMI Vendor-Specific Data Block (HDMI VSDB, H14b-VSDB) */ static void -drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) +drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db, const u8 payload_len) { struct drm_display_info *info = &connector->display_info; - u8 len = cea_db_payload_len(db); info->is_hdmi = true; info->source_physical_address = (db[4] << 8) | db[5]; - if (len >= 6) + if (payload_len >= 6) info->dvi_dual = db[6] & 1; - if (len >= 7) + if (payload_len >= 7) info->max_tmds_clock = db[7] * 5000; /* @@ -6340,14 +6330,14 @@ drm_parse_hdmi_vsdb_video(struct drm_connector *connector, const u8 *db) * HDMI infoframe support was first added in HDMI 1.4. Assume the sink * supports infoframes if HDMI_Video_present is set. */ - if (len >= 8 && db[8] & BIT(5)) + if (payload_len >= 8 && db[8] & BIT(5)) info->has_hdmi_infoframe = true; drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] HDMI: DVI dual %d, max TMDS clock %d kHz\n", connector->base.id, connector->name, info->dvi_dual, info->max_tmds_clock); - drm_parse_hdmi_deep_color_info(connector, db); + drm_parse_hdmi_deep_color_info(connector, db, payload_len); } /* @@ -6410,12 +6400,13 @@ static void drm_parse_cea_ext(struct drm_connector *connector, cea_db_iter_for_each(db, &iter) { /* FIXME: convert parsers to use struct cea_db */ const u8 *data = (const u8 *)db; + int len = cea_db_payload_len(db); if (cea_db_is_hdmi_vsdb(db)) - drm_parse_hdmi_vsdb_video(connector, data); + drm_parse_hdmi_vsdb_video(connector, data, len); else if (cea_db_is_hdmi_forum_vsdb(db) || cea_db_is_hdmi_forum_scdb(db)) - drm_parse_hdmi_forum_scds(connector, data); + drm_parse_hdmi_forum_scds(connector, data, len); else if (cea_db_is_microsoft_vsdb(db)) drm_parse_microsoft_vsdb(connector, data); else if (cea_db_is_y420cmdb(db)) @@ -6425,7 +6416,7 @@ static void drm_parse_cea_ext(struct drm_connector *connector, else if (cea_db_is_vcdb(db)) drm_parse_vcdb(connector, data); else if (cea_db_is_hdmi_hdr_metadata_block(db)) - drm_parse_hdr_metadata_block(connector, data); + drm_parse_hdr_metadata_block(connector, data, len); else if (cea_db_tag(db) == CTA_DB_VIDEO) parse_cta_vdb(connector, db); else if (cea_db_tag(db) == CTA_DB_AUDIO)
Address the FIXME in cea_db_payload_len Transition to passing struct cea_db * everywhere Precompute the payload length in drm_parse_cea_ext and pass to individual parsers to avoid casting struct cea_db pointer to u8 pointer where length is needed. Since the type of payload length is inconsistent in the file, use u8, u16 where it was already in place, use int elsewhere. Signed-off-by: Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com> --- drivers/gpu/drm/drm_edid.c | 63 ++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 36 deletions(-) base-commit: 1d227fcc72223cbdd34d0ce13541cbaab5e0d72f