Re: [Intel-gfx] [PATCH v7 02/22] drm/edid: fix parsing of 3D modes from HDMI VSDB

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Jan 04, 2023 at 12:05:17PM +0200, Jani Nikula wrote:
> Commit 537d9ed2f6c1 ("drm/edid: convert add_cea_modes() to use cea db
> iter") inadvertently moved the do_hdmi_vsdb_modes() call within the db
> iteration loop, always passing NULL as the CTA VDB to
> do_hdmi_vsdb_modes(), skipping a lot of stereo modes.
> 
> Move the call back outside of the loop.
> 
> This does mean only one CTA VDB and HDMI VSDB combination will be
> handled, but it's an unlikely scenario to have more than one of either
> block, and it was not accounted for before the regression either.
> 
> Fixes: 537d9ed2f6c1 ("drm/edid: convert add_cea_modes() to use cea db iter")
> Cc: <stable@vger.kernel.org> # v6.0+
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 23f7146e6a9b..b94adb9bbefb 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5249,13 +5249,12 @@ static int add_cea_modes(struct drm_connector *connector,
>  {
>  	const struct cea_db *db;
>  	struct cea_db_iter iter;
> +	const u8 *hdmi = NULL, *video = NULL;
> +	u8 hdmi_len = 0, video_len = 0;
>  	int modes = 0;
>  
>  	cea_db_iter_edid_begin(drm_edid, &iter);
>  	cea_db_iter_for_each(db, &iter) {
> -		const u8 *hdmi = NULL, *video = NULL;
> -		u8 hdmi_len = 0, video_len = 0;
> -
>  		if (cea_db_tag(db) == CTA_DB_VIDEO) {
>  			video = cea_db_data(db);
>  			video_len = cea_db_payload_len(db);
> @@ -5271,18 +5270,17 @@ static int add_cea_modes(struct drm_connector *connector,
>  			modes += do_y420vdb_modes(connector, vdb420,
>  						  cea_db_payload_len(db) - 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);
>  	}
>  	cea_db_iter_end(&iter);
>  
> +	/*
> +	 * 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);

I wonder if there are any EDIDs with multiple copies
of either data block... But the original code couldn't
deal with that either so not really a concern for this
patch.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
>  	return modes;
>  }
>  
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel