On Tue, Mar 03, 2015 at 12:49:43PM +0100, Philipp Zabel wrote:
> 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.
I have no objections to keeping the current prototype. It's something we
can always fixup if we want to. Also keeping the symmetry with min/max
values for hactive and vactive is okay in my opinion.
Were there any other remaining points? If not I'll just apply this as
is.
Thierry
Attachment:
pgp9mUJV51Zwv.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
