Hi, On Sun, Jun 30, 2019 at 1:55 PM Sam Ravnborg <sam@xxxxxxxxxxxx> wrote: > > Hi Douglas. > > > > + > > > + /* Only add timings if override was not there or failed to validate */ > > > + if (num == 0 && panel->desc->num_timings) > > > + num = panel_simple_get_timings_modes(panel); > > > + > > > + /* > > > + * Only add fixed modes if timings/override added no mode. > > > > This part I fail to understand. > > If we have a panel where we in panel-simple have specified the timings, > > and done so using display_timing so with proper {min, typ, max} then it > > should be perfectly legal to specify a more precise variant in the DT > > file. > > Or what did I miss here? > > Got it now. > If display_mode is used for timings this is what you call "fixed mode". > Hmm, if I got confused someone else may also be confused by this naming. The name "fixed mode" comes from the old code, though I guess in the old code it used to refer to a mode that came from either the display_timing or the display_mode. How about if I call it "panel_simple_get_from_fixed_display_mode"? ...or if you have another suggestion feel free to chime in. NOTE: Since this feedback is minor and this patch has been outstanding for a while (and is blocking other work), I am assuming that the best path forward is for Heiko to land this patch with Thierry's Ack and I'll send a follow-up. Please yell if you disagree. I'll respond to each of your emails separately and then wait for your response before sending the follow-up series. -Doug