diff mbox series

[v2,2/5] drm/edid: Abstract away cea_edid_modes[]

Message ID 20190711103234.6294-3-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/edid: Add new modes from CTA-861-G | expand

Commit Message

Ville Syrjälä July 11, 2019, 10:32 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're going to need two cea mode tables (on for VICs < 128,
another one for VICs >= 193). To that end replace the direct
edid_cea_modes[] lookups with a function call. And we'll rename
the array to edid_cea_modes_0[] to indicathe how it's to be
indexed.

Cc: Hans Verkuil <hansverk@cisco.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 80 +++++++++++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 22 deletions(-)

Comments

Sharma, Shashank July 19, 2019, 8:45 a.m. UTC | #1 
Hi Ville,

On 7/11/2019 4:02 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We're going to need two cea mode tables (on for VICs < 128,
> another one for VICs >= 193). To that end replace the direct
> edid_cea_modes[] lookups with a function call. And we'll rename
> the array to edid_cea_modes_0[] to indicathe how it's to be
> indexed.

I am not very sure if its a good idea to split into two different 
tables, or adding multiple dummy placeholder modes (like VIC 0) for the 
modes not available. It could give us continuity and we can keep on 
using almost the same functions. We can just add some checks for range 
between VIC 128 - 193. Just a thought.

- Shashank

> Cc: Hans Verkuil <hansverk@cisco.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/drm_edid.c | 80 +++++++++++++++++++++++++++-----------
>   1 file changed, 58 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index bcd9ed569d64..703d2bc26fd9 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -707,12 +707,11 @@ static const struct minimode extra_modes[] = {
>   };
>   
>   /*
> - * Probably taken from CEA-861 spec.
> - * This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c.
> + * From CEA/CTA-861 spec.
>    *
> - * Index using the VIC.
> + * Index with VIC.
>    */
> -static const struct drm_display_mode edid_cea_modes[] = {
> +static const struct drm_display_mode edid_cea_modes_0[] = {
>   	/* 0 - dummy, VICs start at 1 */
>   	{ },
>   	/* 1 - 640x480@60Hz 4:3 */
> @@ -3067,6 +3066,25 @@ static u8 *drm_find_cea_extension(const struct edid *edid)
>   	return cea;
>   }
>   
> +static const struct drm_display_mode *cea_mode_for_vic(u8 vic)
> +{
> +	if (!vic)
> +		return NULL;
> +	if (vic < ARRAY_SIZE(edid_cea_modes_0))
> +		return &edid_cea_modes_0[vic];
> +	return NULL;
> +}
> +
> +static u8 cea_num_vics(void)
> +{
> +	return ARRAY_SIZE(edid_cea_modes_0);
> +}
> +
> +static u8 cea_next_vic(u8 vic)
> +{
> +	return vic + 1;
> +}
> +
>   /*
>    * Calculate the alternate clock for the CEA mode
>    * (60Hz vs. 59.94Hz etc.)
> @@ -3104,14 +3122,14 @@ cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode)
>   	 * get the other variants by simply increasing the
>   	 * vertical front porch length.
>   	 */
> -	BUILD_BUG_ON(edid_cea_modes[8].vtotal != 262 ||
> -		     edid_cea_modes[9].vtotal != 262 ||
> -		     edid_cea_modes[12].vtotal != 262 ||
> -		     edid_cea_modes[13].vtotal != 262 ||
> -		     edid_cea_modes[23].vtotal != 312 ||
> -		     edid_cea_modes[24].vtotal != 312 ||
> -		     edid_cea_modes[27].vtotal != 312 ||
> -		     edid_cea_modes[28].vtotal != 312);
> +	BUILD_BUG_ON(cea_mode_for_vic(8)->vtotal != 262 ||
> +		     cea_mode_for_vic(9)->vtotal != 262 ||
> +		     cea_mode_for_vic(12)->vtotal != 262 ||
> +		     cea_mode_for_vic(13)->vtotal != 262 ||
> +		     cea_mode_for_vic(23)->vtotal != 312 ||
> +		     cea_mode_for_vic(24)->vtotal != 312 ||
> +		     cea_mode_for_vic(27)->vtotal != 312 ||
> +		     cea_mode_for_vic(28)->vtotal != 312);
>   
>   	if (((vic == 8 || vic == 9 ||
>   	      vic == 12 || vic == 13) && mode->vtotal < 263) ||
> @@ -3139,10 +3157,16 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m
>   	if (to_match->picture_aspect_ratio)
>   		match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
>   
> -	for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> -		struct drm_display_mode cea_mode = edid_cea_modes[vic];
> +	for (vic = 1; vic < cea_num_vics(); vic = cea_next_vic(vic)) {
> +		const struct drm_display_mode *mode = cea_mode_for_vic(vic);
> +		struct drm_display_mode cea_mode;
>   		unsigned int clock1, clock2;
>   
> +		if (!mode)
> +			continue;
> +
> +		cea_mode = *mode;
> +
>   		/* Check both 60Hz and 59.94Hz */
>   		clock1 = cea_mode.clock;
>   		clock2 = cea_mode_alternate_clock(&cea_mode);
> @@ -3178,10 +3202,16 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
>   	if (to_match->picture_aspect_ratio)
>   		match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
>   
> -	for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> -		struct drm_display_mode cea_mode = edid_cea_modes[vic];
> +	for (vic = 1; vic < cea_num_vics(); vic = cea_next_vic(vic)) {
> +		const struct drm_display_mode *mode = cea_mode_for_vic(vic);
> +		struct drm_display_mode cea_mode;
>   		unsigned int clock1, clock2;
>   
> +		if (!mode)
> +			continue;
> +
> +		cea_mode = *mode;
> +
>   		/* Check both 60Hz and 59.94Hz */
>   		clock1 = cea_mode.clock;
>   		clock2 = cea_mode_alternate_clock(&cea_mode);
> @@ -3202,7 +3232,7 @@ EXPORT_SYMBOL(drm_match_cea_mode);
>   
>   static bool drm_valid_cea_vic(u8 vic)
>   {
> -	return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes);
> +	return cea_mode_for_vic(vic) != NULL;
>   }
>   
>   /**
> @@ -3214,7 +3244,13 @@ static bool drm_valid_cea_vic(u8 vic)
>    */
>   enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code)
>   {
> -	return edid_cea_modes[video_code].picture_aspect_ratio;
> +	const struct drm_display_mode *mode;
> +
> +	mode = cea_mode_for_vic(video_code);
> +	if (mode)
> +		return mode->picture_aspect_ratio;
> +
> +	return HDMI_PICTURE_ASPECT_NONE;
>   }
>   EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
>   
> @@ -3323,7 +3359,7 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
>   		unsigned int clock1, clock2;
>   
>   		if (drm_valid_cea_vic(vic)) {
> -			cea_mode = &edid_cea_modes[vic];
> +			cea_mode = cea_mode_for_vic(vic);
>   			clock2 = cea_mode_alternate_clock(cea_mode);
>   		} else {
>   			vic = drm_match_hdmi_mode(mode);
> @@ -3398,7 +3434,7 @@ drm_display_mode_from_vic_index(struct drm_connector *connector,
>   	if (!drm_valid_cea_vic(vic))
>   		return NULL;
>   
> -	newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
> +	newmode = drm_mode_duplicate(dev, cea_mode_for_vic(vic));
>   	if (!newmode)
>   		return NULL;
>   
> @@ -3432,7 +3468,7 @@ static int do_y420vdb_modes(struct drm_connector *connector,
>   		if (!drm_valid_cea_vic(vic))
>   			continue;
>   
> -		newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
> +		newmode = drm_mode_duplicate(dev, cea_mode_for_vic(vic));
>   		if (!newmode)
>   			break;
>   		bitmap_set(hdmi->y420_vdb_modes, vic, 1);
> @@ -4001,7 +4037,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
>   	vic = drm_match_cea_mode_clock_tolerance(mode, 5);
>   	if (drm_valid_cea_vic(vic)) {
>   		type = "CEA";
> -		cea_mode = &edid_cea_modes[vic];
> +		cea_mode = cea_mode_for_vic(vic);
>   		clock1 = cea_mode->clock;
>   		clock2 = cea_mode_alternate_clock(cea_mode);
>   	} else {
Ville Syrjälä July 19, 2019, 11:57 a.m. UTC | #2 
On Fri, Jul 19, 2019 at 02:15:34PM +0530, Sharma, Shashank wrote:
> Hi Ville,
> 
> On 7/11/2019 4:02 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We're going to need two cea mode tables (on for VICs < 128,
> > another one for VICs >= 193). To that end replace the direct
> > edid_cea_modes[] lookups with a function call. And we'll rename
> > the array to edid_cea_modes_0[] to indicathe how it's to be
> > indexed.
> 
> I am not very sure if its a good idea to split into two different 
> tables, or adding multiple dummy placeholder modes (like VIC 0) for the 
> modes not available. It could give us continuity and we can keep on 
> using almost the same functions. We can just add some checks for range 
> between VIC 128 - 193. Just a thought.

These things take 208 bytes each. Blowing away 40 KiB for zeroes
seems quite silly. And with the abstraction in place you don't have
to worry about such implementation details at all.

> 
> - Shashank
> 
> > Cc: Hans Verkuil <hansverk@cisco.com>
> > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/drm_edid.c | 80 +++++++++++++++++++++++++++-----------
> >   1 file changed, 58 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index bcd9ed569d64..703d2bc26fd9 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -707,12 +707,11 @@ static const struct minimode extra_modes[] = {
> >   };
> >   
> >   /*
> > - * Probably taken from CEA-861 spec.
> > - * This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c.
> > + * From CEA/CTA-861 spec.
> >    *
> > - * Index using the VIC.
> > + * Index with VIC.
> >    */
> > -static const struct drm_display_mode edid_cea_modes[] = {
> > +static const struct drm_display_mode edid_cea_modes_0[] = {
> >   	/* 0 - dummy, VICs start at 1 */
> >   	{ },
> >   	/* 1 - 640x480@60Hz 4:3 */
> > @@ -3067,6 +3066,25 @@ static u8 *drm_find_cea_extension(const struct edid *edid)
> >   	return cea;
> >   }
> >   
> > +static const struct drm_display_mode *cea_mode_for_vic(u8 vic)
> > +{
> > +	if (!vic)
> > +		return NULL;
> > +	if (vic < ARRAY_SIZE(edid_cea_modes_0))
> > +		return &edid_cea_modes_0[vic];
> > +	return NULL;
> > +}
> > +
> > +static u8 cea_num_vics(void)
> > +{
> > +	return ARRAY_SIZE(edid_cea_modes_0);
> > +}
> > +
> > +static u8 cea_next_vic(u8 vic)
> > +{
> > +	return vic + 1;
> > +}
> > +
> >   /*
> >    * Calculate the alternate clock for the CEA mode
> >    * (60Hz vs. 59.94Hz etc.)
> > @@ -3104,14 +3122,14 @@ cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode)
> >   	 * get the other variants by simply increasing the
> >   	 * vertical front porch length.
> >   	 */
> > -	BUILD_BUG_ON(edid_cea_modes[8].vtotal != 262 ||
> > -		     edid_cea_modes[9].vtotal != 262 ||
> > -		     edid_cea_modes[12].vtotal != 262 ||
> > -		     edid_cea_modes[13].vtotal != 262 ||
> > -		     edid_cea_modes[23].vtotal != 312 ||
> > -		     edid_cea_modes[24].vtotal != 312 ||
> > -		     edid_cea_modes[27].vtotal != 312 ||
> > -		     edid_cea_modes[28].vtotal != 312);
> > +	BUILD_BUG_ON(cea_mode_for_vic(8)->vtotal != 262 ||
> > +		     cea_mode_for_vic(9)->vtotal != 262 ||
> > +		     cea_mode_for_vic(12)->vtotal != 262 ||
> > +		     cea_mode_for_vic(13)->vtotal != 262 ||
> > +		     cea_mode_for_vic(23)->vtotal != 312 ||
> > +		     cea_mode_for_vic(24)->vtotal != 312 ||
> > +		     cea_mode_for_vic(27)->vtotal != 312 ||
> > +		     cea_mode_for_vic(28)->vtotal != 312);
> >   
> >   	if (((vic == 8 || vic == 9 ||
> >   	      vic == 12 || vic == 13) && mode->vtotal < 263) ||
> > @@ -3139,10 +3157,16 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m
> >   	if (to_match->picture_aspect_ratio)
> >   		match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
> >   
> > -	for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > -		struct drm_display_mode cea_mode = edid_cea_modes[vic];
> > +	for (vic = 1; vic < cea_num_vics(); vic = cea_next_vic(vic)) {
> > +		const struct drm_display_mode *mode = cea_mode_for_vic(vic);
> > +		struct drm_display_mode cea_mode;
> >   		unsigned int clock1, clock2;
> >   
> > +		if (!mode)
> > +			continue;
> > +
> > +		cea_mode = *mode;
> > +
> >   		/* Check both 60Hz and 59.94Hz */
> >   		clock1 = cea_mode.clock;
> >   		clock2 = cea_mode_alternate_clock(&cea_mode);
> > @@ -3178,10 +3202,16 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
> >   	if (to_match->picture_aspect_ratio)
> >   		match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
> >   
> > -	for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > -		struct drm_display_mode cea_mode = edid_cea_modes[vic];
> > +	for (vic = 1; vic < cea_num_vics(); vic = cea_next_vic(vic)) {
> > +		const struct drm_display_mode *mode = cea_mode_for_vic(vic);
> > +		struct drm_display_mode cea_mode;
> >   		unsigned int clock1, clock2;
> >   
> > +		if (!mode)
> > +			continue;
> > +
> > +		cea_mode = *mode;
> > +
> >   		/* Check both 60Hz and 59.94Hz */
> >   		clock1 = cea_mode.clock;
> >   		clock2 = cea_mode_alternate_clock(&cea_mode);
> > @@ -3202,7 +3232,7 @@ EXPORT_SYMBOL(drm_match_cea_mode);
> >   
> >   static bool drm_valid_cea_vic(u8 vic)
> >   {
> > -	return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes);
> > +	return cea_mode_for_vic(vic) != NULL;
> >   }
> >   
> >   /**
> > @@ -3214,7 +3244,13 @@ static bool drm_valid_cea_vic(u8 vic)
> >    */
> >   enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code)
> >   {
> > -	return edid_cea_modes[video_code].picture_aspect_ratio;
> > +	const struct drm_display_mode *mode;
> > +
> > +	mode = cea_mode_for_vic(video_code);
> > +	if (mode)
> > +		return mode->picture_aspect_ratio;
> > +
> > +	return HDMI_PICTURE_ASPECT_NONE;
> >   }
> >   EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
> >   
> > @@ -3323,7 +3359,7 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
> >   		unsigned int clock1, clock2;
> >   
> >   		if (drm_valid_cea_vic(vic)) {
> > -			cea_mode = &edid_cea_modes[vic];
> > +			cea_mode = cea_mode_for_vic(vic);
> >   			clock2 = cea_mode_alternate_clock(cea_mode);
> >   		} else {
> >   			vic = drm_match_hdmi_mode(mode);
> > @@ -3398,7 +3434,7 @@ drm_display_mode_from_vic_index(struct drm_connector *connector,
> >   	if (!drm_valid_cea_vic(vic))
> >   		return NULL;
> >   
> > -	newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
> > +	newmode = drm_mode_duplicate(dev, cea_mode_for_vic(vic));
> >   	if (!newmode)
> >   		return NULL;
> >   
> > @@ -3432,7 +3468,7 @@ static int do_y420vdb_modes(struct drm_connector *connector,
> >   		if (!drm_valid_cea_vic(vic))
> >   			continue;
> >   
> > -		newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
> > +		newmode = drm_mode_duplicate(dev, cea_mode_for_vic(vic));
> >   		if (!newmode)
> >   			break;
> >   		bitmap_set(hdmi->y420_vdb_modes, vic, 1);
> > @@ -4001,7 +4037,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
> >   	vic = drm_match_cea_mode_clock_tolerance(mode, 5);
> >   	if (drm_valid_cea_vic(vic)) {
> >   		type = "CEA";
> > -		cea_mode = &edid_cea_modes[vic];
> > +		cea_mode = cea_mode_for_vic(vic);
> >   		clock1 = cea_mode->clock;
> >   		clock2 = cea_mode_alternate_clock(cea_mode);
> >   	} else {
Ville Syrjälä July 19, 2019, 12:13 p.m. UTC | #3 
On Fri, Jul 19, 2019 at 02:57:51PM +0300, Ville Syrjälä wrote:
> On Fri, Jul 19, 2019 at 02:15:34PM +0530, Sharma, Shashank wrote:
> > Hi Ville,
> > 
> > On 7/11/2019 4:02 PM, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > We're going to need two cea mode tables (on for VICs < 128,
> > > another one for VICs >= 193). To that end replace the direct
> > > edid_cea_modes[] lookups with a function call. And we'll rename
> > > the array to edid_cea_modes_0[] to indicathe how it's to be
> > > indexed.
> > 
> > I am not very sure if its a good idea to split into two different 
> > tables, or adding multiple dummy placeholder modes (like VIC 0) for the 
> > modes not available. It could give us continuity and we can keep on 
> > using almost the same functions. We can just add some checks for range 
> > between VIC 128 - 193. Just a thought.
> 
> These things take 208 bytes each. Blowing away 40 KiB for zeroes

Also we should probably think about shrinking these things a bit. A
(suspiciously?) easy start would be to s/int/u16/ for most of the
timings. Though not sure how many years we have until 64k displays
become a thing.

Also having an int for {width,height}_mm is probably overkill. Or
at least I haven't heard of anyone having 2000km wide displays yet.
I couldn't even fit one in Finland anyway. u16 would still give
us ~65m which seems plenty.

'type' I think could fit into u8.

Then we have '*private'. Maybe totally unused?

Also not sure if we really need 'name[]'. Maybe we can just generate
it on demand? Might be tied in with the uabi I guess so maybe can't
kill it. Depending on the length of a typical name switching to a
pointer might save some bytes though.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bcd9ed569d64..703d2bc26fd9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -707,12 +707,11 @@  static const struct minimode extra_modes[] = {
 };
 
 /*
- * Probably taken from CEA-861 spec.
- * This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c.
+ * From CEA/CTA-861 spec.
  *
- * Index using the VIC.
+ * Index with VIC.
  */
-static const struct drm_display_mode edid_cea_modes[] = {
+static const struct drm_display_mode edid_cea_modes_0[] = {
 	/* 0 - dummy, VICs start at 1 */
 	{ },
 	/* 1 - 640x480@60Hz 4:3 */
@@ -3067,6 +3066,25 @@  static u8 *drm_find_cea_extension(const struct edid *edid)
 	return cea;
 }
 
+static const struct drm_display_mode *cea_mode_for_vic(u8 vic)
+{
+	if (!vic)
+		return NULL;
+	if (vic < ARRAY_SIZE(edid_cea_modes_0))
+		return &edid_cea_modes_0[vic];
+	return NULL;
+}
+
+static u8 cea_num_vics(void)
+{
+	return ARRAY_SIZE(edid_cea_modes_0);
+}
+
+static u8 cea_next_vic(u8 vic)
+{
+	return vic + 1;
+}
+
 /*
  * Calculate the alternate clock for the CEA mode
  * (60Hz vs. 59.94Hz etc.)
@@ -3104,14 +3122,14 @@  cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode)
 	 * get the other variants by simply increasing the
 	 * vertical front porch length.
 	 */
-	BUILD_BUG_ON(edid_cea_modes[8].vtotal != 262 ||
-		     edid_cea_modes[9].vtotal != 262 ||
-		     edid_cea_modes[12].vtotal != 262 ||
-		     edid_cea_modes[13].vtotal != 262 ||
-		     edid_cea_modes[23].vtotal != 312 ||
-		     edid_cea_modes[24].vtotal != 312 ||
-		     edid_cea_modes[27].vtotal != 312 ||
-		     edid_cea_modes[28].vtotal != 312);
+	BUILD_BUG_ON(cea_mode_for_vic(8)->vtotal != 262 ||
+		     cea_mode_for_vic(9)->vtotal != 262 ||
+		     cea_mode_for_vic(12)->vtotal != 262 ||
+		     cea_mode_for_vic(13)->vtotal != 262 ||
+		     cea_mode_for_vic(23)->vtotal != 312 ||
+		     cea_mode_for_vic(24)->vtotal != 312 ||
+		     cea_mode_for_vic(27)->vtotal != 312 ||
+		     cea_mode_for_vic(28)->vtotal != 312);
 
 	if (((vic == 8 || vic == 9 ||
 	      vic == 12 || vic == 13) && mode->vtotal < 263) ||
@@ -3139,10 +3157,16 @@  static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m
 	if (to_match->picture_aspect_ratio)
 		match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
 
-	for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
-		struct drm_display_mode cea_mode = edid_cea_modes[vic];
+	for (vic = 1; vic < cea_num_vics(); vic = cea_next_vic(vic)) {
+		const struct drm_display_mode *mode = cea_mode_for_vic(vic);
+		struct drm_display_mode cea_mode;
 		unsigned int clock1, clock2;
 
+		if (!mode)
+			continue;
+
+		cea_mode = *mode;
+
 		/* Check both 60Hz and 59.94Hz */
 		clock1 = cea_mode.clock;
 		clock2 = cea_mode_alternate_clock(&cea_mode);
@@ -3178,10 +3202,16 @@  u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
 	if (to_match->picture_aspect_ratio)
 		match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
 
-	for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
-		struct drm_display_mode cea_mode = edid_cea_modes[vic];
+	for (vic = 1; vic < cea_num_vics(); vic = cea_next_vic(vic)) {
+		const struct drm_display_mode *mode = cea_mode_for_vic(vic);
+		struct drm_display_mode cea_mode;
 		unsigned int clock1, clock2;
 
+		if (!mode)
+			continue;
+
+		cea_mode = *mode;
+
 		/* Check both 60Hz and 59.94Hz */
 		clock1 = cea_mode.clock;
 		clock2 = cea_mode_alternate_clock(&cea_mode);
@@ -3202,7 +3232,7 @@  EXPORT_SYMBOL(drm_match_cea_mode);
 
 static bool drm_valid_cea_vic(u8 vic)
 {
-	return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes);
+	return cea_mode_for_vic(vic) != NULL;
 }
 
 /**
@@ -3214,7 +3244,13 @@  static bool drm_valid_cea_vic(u8 vic)
  */
 enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code)
 {
-	return edid_cea_modes[video_code].picture_aspect_ratio;
+	const struct drm_display_mode *mode;
+
+	mode = cea_mode_for_vic(video_code);
+	if (mode)
+		return mode->picture_aspect_ratio;
+
+	return HDMI_PICTURE_ASPECT_NONE;
 }
 EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
 
@@ -3323,7 +3359,7 @@  add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid)
 		unsigned int clock1, clock2;
 
 		if (drm_valid_cea_vic(vic)) {
-			cea_mode = &edid_cea_modes[vic];
+			cea_mode = cea_mode_for_vic(vic);
 			clock2 = cea_mode_alternate_clock(cea_mode);
 		} else {
 			vic = drm_match_hdmi_mode(mode);
@@ -3398,7 +3434,7 @@  drm_display_mode_from_vic_index(struct drm_connector *connector,
 	if (!drm_valid_cea_vic(vic))
 		return NULL;
 
-	newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
+	newmode = drm_mode_duplicate(dev, cea_mode_for_vic(vic));
 	if (!newmode)
 		return NULL;
 
@@ -3432,7 +3468,7 @@  static int do_y420vdb_modes(struct drm_connector *connector,
 		if (!drm_valid_cea_vic(vic))
 			continue;
 
-		newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
+		newmode = drm_mode_duplicate(dev, cea_mode_for_vic(vic));
 		if (!newmode)
 			break;
 		bitmap_set(hdmi->y420_vdb_modes, vic, 1);
@@ -4001,7 +4037,7 @@  static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode)
 	vic = drm_match_cea_mode_clock_tolerance(mode, 5);
 	if (drm_valid_cea_vic(vic)) {
 		type = "CEA";
-		cea_mode = &edid_cea_modes[vic];
+		cea_mode = cea_mode_for_vic(vic);
 		clock1 = cea_mode->clock;
 		clock2 = cea_mode_alternate_clock(cea_mode);
 	} else {