[PATCH 2/2 v6] of: add generic videomode description

[Thierry Reding]

On Tue, Oct 09, 2012 at 09:26:08AM +0200, Steffen Trumtrar wrote:
> Hi Laurent,
> 
> On Mon, Oct 08, 2012 at 10:52:04PM +0200, Laurent Pinchart wrote:
> > Hi Steffen,
> > 
> > On Monday 08 October 2012 14:48:01 Steffen Trumtrar wrote:
> > > On Mon, Oct 08, 2012 at 02:13:50PM +0200, Laurent Pinchart wrote:
> > > > On Thursday 04 October 2012 19:59:20 Steffen Trumtrar wrote:
[...]
> > > > > +int of_get_videomode(struct device_node *np, struct videomode *vm, int
> > > > > index)
> > > > 
> > > > I wonder how to avoid abuse of this functions. It's a useful helper for
> > > > drivers that need to get a video mode once only, but would result in lower
> > > > performances if a driver calls it for every mode. Drivers must call
> > > > of_get_display_timing_list instead in that case and case the display
> > > > timings. I'm wondering whether we should really expose of_get_videomode.
> > > 
> > > The intent was to let the driver decide. That way all the other overhead may
> > > be skipped.
> > 
> > My point is that driver writers might just call of_get_videomode() in a loop, 
> > not knowing that it's expensive. I want to avoid that. We need to at least add 
> > kerneldoc to the function stating that this shouldn't be done.
> > 
> 
> You're right. That should be made clear in the code.

In that case we should export videomode_from_timing(). For Tegra DRM I
wrote a small utility function that takes a struct display_timings and
converts every timing to a struct videomode which is then converted to
a struct drm_display_mode and added to the DRM connector. The code is
really generic and could be part of the DRM core.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121020/633e65d8/attachment.pgp>