On Saturday, February 26, 2022 1:47 AM, Ville Syrjälä wrote:
>On Thu, Feb 24, 2022 at 10:16:23PM +0800, Lee Shawn C wrote:
>> Try to find and parse more CEA ext blocks if edid->extensions is
>> greater than one.
>>
>> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
>> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
>> Signed-off-by: Lee Shawn C <shawn.c.lee@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/drm_edid.c | 110
>> ++++++++++++++++++++-----------------
>> 1 file changed, 60 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index a504542238ed..19426f28b411 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3353,16 +3353,14 @@ const u8 *drm_find_edid_extension(const struct edid *edid,
>> return edid_ext;
>> }
>>
>> -static const u8 *drm_find_cea_extension(const struct edid *edid)
>> +static const u8 *drm_find_cea_extension(const struct edid *edid, int
>> +*ext_index)
>> {
>> const struct displayid_block *block;
>> struct displayid_iter iter;
>> const u8 *cea;
>> - int ext_index = 0;
>>
>> - /* Look for a top level CEA extension block */
>> - /* FIXME: make callers iterate through multiple CEA ext blocks? */
>> - cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index);
>> + /* Look for a CEA extension block from ext_index */
>> + cea = drm_find_edid_extension(edid, CEA_EXT, ext_index);
>> if (cea)
>> return cea;
>>
>> @@ -3643,10 +3641,10 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>> struct drm_device *dev = connector->dev;
>> struct drm_display_mode *mode, *tmp;
>> LIST_HEAD(list);
>> - int modes = 0;
>> + int modes = 0, ext_index = 0;
>>
>> /* Don't add CEA modes if the CEA extension block is missing */
>> - if (!drm_find_cea_extension(edid))
>> + if (!drm_find_cea_extension(edid, &ext_index))
>> return 0;
>>
>> /*
>> @@ -4321,46 +4319,58 @@ static void drm_parse_y420cmdb_bitmap(struct
>> drm_connector *connector, static int add_cea_modes(struct
>> drm_connector *connector, struct edid *edid) {
>> - const u8 *cea = drm_find_cea_extension(edid);
>> + const u8 *cea = NULL;
>> const u8 *db, *hdmi = NULL, *video = NULL;
>> u8 dbl, hdmi_len, video_len = 0;
>> - int modes = 0;
>> + int modes = 0, ext_index = 0;
>>
>> - if (cea && cea_revision(cea) >= 3) {
>> - int i, start, end;
>> + for (;;) {
>> + cea = drm_find_cea_extension(edid, &ext_index);
>
>Please split this into two patches:
>1. do the *ext_index stuff
>2. do the loop
>
Thanks for comment! Will split this patch to two in next version.
>>
>> - if (cea_db_offsets(cea, &start, &end))
>> - return 0;
>> + if (!cea)
>> + break;
>>
>> - for_each_cea_db(cea, i, start, end) {
>> - db = &cea[i];
>> - dbl = cea_db_payload_len(db);
>> + if (cea && cea_revision(cea) >= 3) {
>> + int i, start, end;
>> +
>> + if (cea_db_offsets(cea, &start, &end))
>> + continue;
>>
>> - if (cea_db_tag(db) == VIDEO_BLOCK) {
>> - video = db + 1;
>> - video_len = dbl;
>> - modes += do_cea_modes(connector, video, dbl);
>> - } else if (cea_db_is_hdmi_vsdb(db)) {
>> - hdmi = db;
>> - hdmi_len = dbl;
>> - } else if (cea_db_is_y420vdb(db)) {
>> - const u8 *vdb420 = &db[2];
>> -
>> - /* Add 4:2:0(only) modes present in EDID */
>> - modes += do_y420vdb_modes(connector,
>> - vdb420,
>> - dbl - 1);
>> + for_each_cea_db(cea, i, start, end) {
>> + db = &cea[i];
>> + dbl = cea_db_payload_len(db);
>> +
>> + if (cea_db_tag(db) == VIDEO_BLOCK) {
>> + video = db + 1;
>> + video_len = dbl;
>> + modes += do_cea_modes(connector, video, dbl);
>> + } else if (cea_db_is_hdmi_vsdb(db)) {
>> + hdmi = db;
>> + hdmi_len = dbl;
>> + } else if (cea_db_is_y420vdb(db)) {
>> + const u8 *vdb420 = &db[2];
>> +
>> + /* Add 4:2:0(only) modes present in EDID */
>> + modes += do_y420vdb_modes(connector,
>> + vdb420,
>> + dbl - 1);
>> + }
>> }
>> }
>> - }
>>
>> - /*
>> - * We parse the HDMI VSDB after having added the cea modes as we will
>> - * be patching their flags when the sink supports stereo 3D.
>> - */
>> - if (hdmi)
>> - modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len, video,
>> - video_len);
>> + /*
>> + * We parse the HDMI VSDB after having added the cea modes as we will
>> + * be patching their flags when the sink supports stereo 3D.
>> + */
>> + if (hdmi) {
>
>Looks like you're potentially using stale information here from previous loops. Please move all the possible variables to a tighter scope so this can't happen.
>
Yes, you are right! I will modify these codes and send patch v2 for review later.
Best regards,
Shawn
>> + modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len, video,
>> + video_len);
>> + hdmi = NULL;
>> + video = NULL;
>> + hdmi_len = 0;
>> + video_len = 0;
>> + }
>> + }
>>
>> return modes;
>> }
>> @@ -4562,7 +4572,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>> uint8_t *eld = connector->eld;
>> const u8 *cea;
>> const u8 *db;
>> - int total_sad_count = 0;
>> + int total_sad_count = 0, ext_index = 0;
>> int mnl;
>> int dbl;
>>
>> @@ -4571,7 +4581,7 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>> if (!edid)
>> return;
>>
>> - cea = drm_find_cea_extension(edid);
>> + cea = drm_find_cea_extension(edid, &ext_index);
>> if (!cea) {
>> DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
>> return;
>> @@ -4655,11 +4665,11 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>> */
>> int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads) {
>> - int count = 0;
>> + int count = 0, ext_index = 0;
>> int i, start, end, dbl;
>> const u8 *cea;
>>
>> - cea = drm_find_cea_extension(edid);
>> + cea = drm_find_cea_extension(edid, &ext_index);
>> if (!cea) {
>> DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
>> return 0;
>> @@ -4717,11 +4727,11 @@ EXPORT_SYMBOL(drm_edid_to_sad);
>> */
>> int drm_edid_to_speaker_allocation(struct edid *edid, u8 **sadb) {
>> - int count = 0;
>> + int count = 0, ext_index = 0;
>> int i, start, end, dbl;
>> const u8 *cea;
>>
>> - cea = drm_find_cea_extension(edid);
>> + cea = drm_find_cea_extension(edid, &ext_index);
>> if (!cea) {
>> DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
>> return 0;
>> @@ -4813,10 +4823,10 @@ EXPORT_SYMBOL(drm_av_sync_delay); bool
>> drm_detect_hdmi_monitor(struct edid *edid) {
>> const u8 *edid_ext;
>> - int i;
>> + int i, ext_index = 0;
>> int start_offset, end_offset;
>>
>> - edid_ext = drm_find_cea_extension(edid);
>> + edid_ext = drm_find_cea_extension(edid, &ext_index);
>> if (!edid_ext)
>> return false;
>>
>> @@ -4851,11 +4861,11 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor);
>> bool drm_detect_monitor_audio(struct edid *edid) {
>> const u8 *edid_ext;
>> - int i, j;
>> + int i, j, ext_index = 0;
>> bool has_audio = false;
>> int start_offset, end_offset;
>>
>> - edid_ext = drm_find_cea_extension(edid);
>> + edid_ext = drm_find_cea_extension(edid, &ext_index);
>> if (!edid_ext)
>> goto end;
>>
>> @@ -5177,9 +5187,9 @@ static void drm_parse_cea_ext(struct
>> drm_connector *connector, {
>> struct drm_display_info *info = &connector->display_info;
>> const u8 *edid_ext;
>> - int i, start, end;
>> + int i, start, end, ext_index = 0;
>>
>> - edid_ext = drm_find_cea_extension(edid);
>> + edid_ext = drm_find_cea_extension(edid, &ext_index);
>> if (!edid_ext)
>> return;
>>
>> --
>> 2.17.1
>
>--
>Ville Syrjälä
>Intel
>
