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

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

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.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx