Re: [PATCH 1/3] drm/panel: Add display_timing support

[Philipp Zabel <p.zabel@pengutronix.de>]

Hi Thierry,

Am Montag, den 23.02.2015, 15:04 +0100 schrieb Philipp Zabel:
> Hi Thierry,
> 
> do you have any further thoughts on this?

[...]
> > Sorry for taking so long to get back on this. I generally like the idea,
> > though a couple of things are unclear to me.
> > 
> > In struct display_timing we have:
> > 
> > 	struct timing_entry hactive;
> > 	...
> > 	struct timing_entry vactive;
> > 
> > I wonder if that really makes sense. Are there really datasheets that
> > provide a valid range for the number of horizontal and vertical active
> > pixels?
> 
> I could send a patch to change hactive/vactive to a single value
> and change Documentation/devicetree/bindings/video/display-timing.txt
> to clarify ranges are not allowed for these properties until somebody
> digs out a panel that actually needs this.
> Adding Steffen to Cc: in case there was a reason other than symmetry.

That seems not to be the case so far.

[...]
> > >  /**
> > >   * struct drm_panel_funcs - perform operations on a given panel
> > > @@ -38,6 +39,8 @@ struct drm_panel;
> > >   * @enable: enable panel (turn on back light, etc.)
> > >   * @get_modes: add modes to the connector that the panel is attached to and
> > >   * return the number of modes added
> > > + * @get_timings: copy display timings into the provided array and return
> > > + * the number of display timings available
> > >   *
> > >   * The .prepare() function is typically called before the display controller
> > >   * starts to transmit video data. Panel drivers can use this to turn the panel
> > > @@ -68,6 +71,8 @@ struct drm_panel_funcs {
> > >  	int (*prepare)(struct drm_panel *panel);
> > >  	int (*enable)(struct drm_panel *panel);
> > >  	int (*get_modes)(struct drm_panel *panel);
> > > +	int (*get_timings)(struct drm_panel *panel, unsigned int num_timings,
> > > +			   struct display_timing *timings);
> > 
> > Also are there even panels that support more than one set of timings?
> > I've never heard of panels that are actually able to do more than one
> > mode, so I'm wondering if num_timings isn't overdoing it a little here.
> > I guess it doesn't hurt all that much, though.
> 
> Would you prefer
> 	struct display_timing *(*get_timing)(struct drm_panel *panel);
> ?

I'd like to resend this. Please let me know if you want me to change
this function prototype.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel