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: <sta...@vger.kernel.org> # v6.0+
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nik...@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.syrj...@linux.intel.com>
> +
> return modes;
> }
>
> --
> 2.34.1
--
Ville Syrjälä
Intel
