Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [RFC v2 0/5] Common Display Framework
From: Rob Clark @ 2012-12-19 16:05 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Thomas Petazzoni, Linux Fbdev development list, Benjamin Gaignard,
	Tom Gall, dri-devel@lists.freedesktop.org, Kyungmin Park,
	Ragesh Radhakrishnan, Laurent Pinchart, Philipp Zabel,
	Maxime Ripard, Vikas Sajjan, Sumit Semwal, Sebastien Guiriec,
	linux-media@vger.kernel.org
In-Reply-To: <50D1DF42.3070008@ti.com>

On Wed, Dec 19, 2012 at 9:37 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 2012-12-19 17:26, Rob Clark wrote:
>>
>> And, there are also external HDMI encoders (for example connected over
>> i2c) that can also be shared between boards.  So I think there will be
>> a number of cases where CDF is appropriate for HDMI drivers.  Although
>> trying to keep this all independent of DRM (as opposed to just
>> something similar to what drivers/gpu/i2c is today) seems a bit
>> overkill for me.  Being able to use the helpers in drm and avoiding an
>> extra layer of translation seems like the better option to me.  So my
>> vote would be drivers/gpu/cdf.
>
> Well, we need to think about that. I would like to keep CDF independent
> of DRM. I don't like tying different components/frameworks together if
> there's no real need for that.
>
> Also, something that Laurent mentioned in our face-to-face discussions:
> Some IPs/chips can be used for other purposes than with DRM.
>
> He had an example of a board, that (if I understood right) gets video
> signal from somewhere outside the board, processes the signal with some
> IPs/chips, and then outputs the signal. So there's no framebuffer, and
> the image is not stored anywhere. I think the framework used in these
> cases is always v4l2.
>
> The IPs/chips in the above model may be the exact same IPs/chips that
> are used with "normal" display. If the CDF was tied to DRM, using the
> same drivers for normal and these streaming cases would probably not be
> possible.

Well, maybe there is a way, but it really seems to be
over-complicating things unnecessarily to keep CDF independent of
DRM..  there will be a lot more traditional uses of CDF compared to
one crazy use-case.  So I don't really fancy making it more difficult
than in needs to be for everyone.

Probably the thing to do is take a step back and reconsider that one
crazy use-case.  For example, KMS doesn't enforce that the buffer
handled passed when you create a drm framebuffer object to scan out is
a GEM buffer.  So on that one crazy platform, maybe it makes sense to
have a DRM/KMS display driver that takes a handle to identify which
video stream coming from the capture end of the pipeline.  Anyways,
that is just an off-the-top-of-my-head idea, probably there are other
options too.

BR,
-R

>  Tomi
>
>

^ permalink raw reply

* Re: [RFC v2 0/5] Common Display Framework
From: Tomi Valkeinen @ 2012-12-19 15:37 UTC (permalink / raw)
  To: Rob Clark
  Cc: Jani Nikula, Laurent Pinchart, Thomas Petazzoni,
	Linux Fbdev development list, Philipp Zabel, Tom Gall,
	Ragesh Radhakrishnan, dri-devel@lists.freedesktop.org,
	Kyungmin Park, Benjamin Gaignard, Bryan Wu, Maxime Ripard,
	Vikas Sajjan, Sumit Semwal, Sebastien Guiriec,
	linux-media@vger.kernel.org
In-Reply-To: <CAF6AEGuSt0CL2sFGK-PZnw6+r9zhGHO4CEjJEWaR8eGhks2=UQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4348 bytes --]

On 2012-12-19 17:26, Rob Clark wrote:
> On Wed, Dec 19, 2012 at 8:57 AM, Jani Nikula
> <jani.nikula@linux.intel.com> wrote:
>>
>> Hi Laurent -
>>
>> On Tue, 18 Dec 2012, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>>> Hi Jani,
>>>
>>> On Monday 17 December 2012 18:53:37 Jani Nikula wrote:
>>>> I can see the need for a framework for DSI panels and such (in fact Tomi
>>>> and I have talked about it like 2-3 years ago already!) but what is the
>>>> story for HDMI and DP? In particular, what's the relationship between
>>>> DRM and CDF here? Is there a world domination plan to switch the DRM
>>>> drivers to use this framework too? ;) Do you have some rough plans how
>>>> DRM and CDF should work together in general?
>>>
>>> There's always a world domination plan, isn't there ? :-)
>>>
>>> I certainly want CDF to be used by DRM (or more accurately KMS). That's what
>>> the C stands for, common refers to sharing panel and other display entity
>>> drivers between FBDEV, KMS and V4L2.
>>>
>>> I currently have no plan to expose CDF internals to userspace through the KMS
>>> API. We might have to do so later if the hardware complexity grows in such a
>>> way that finer control than what KMS provides needs to be exposed to
>>> userspace, but I don't think we're there yet. The CDF API will thus only be
>>> used internally in the kernel by display controller drivers. The KMS core
>>> might get functions to handle common display entity operations, but the bulk
>>> of the work will be in the display controller drivers to start with. We will
>>> then see what can be abstracted in KMS helper functions.
>>>
>>> Regarding HDMI and DP, I imagine HDMI and DP drivers that would use the CDF
>>> API. That's just a thought for now, I haven't tried to implement them, but it
>>> would be nice to handle HDMI screens and DPI/DBI/DSI panels in a generic way.
>>>
>>> Do you have thoughts to share on this topic ?
>>
>> It just seems to me that, at least from a DRM/KMS perspective, adding
>> another layer (=CDF) for HDMI or DP (or legacy outputs) would be
>> overengineering it. They are pretty well standardized, and I don't see
>> there would be a need to write multiple display drivers for them. Each
>> display controller has one, and can easily handle any chip specific
>> requirements right there. It's my gut feeling that an additional
>> framework would just get in the way. Perhaps there could be more common
>> HDMI/DP helper style code in DRM to reduce overlap across KMS drivers,
>> but that's another thing.
>>
>> So is the HDMI/DP drivers using CDF a more interesting idea from a
>> non-DRM perspective? Or, put another way, is it more of an alternative
>> to using DRM? Please enlighten me if there's some real benefit here that
>> I fail to see!
> 
> fwiw, I think there are at least a couple cases where multiple SoC's
> have the same HDMI IP block.
> 
> And, there are also external HDMI encoders (for example connected over
> i2c) that can also be shared between boards.  So I think there will be
> a number of cases where CDF is appropriate for HDMI drivers.  Although
> trying to keep this all independent of DRM (as opposed to just
> something similar to what drivers/gpu/i2c is today) seems a bit
> overkill for me.  Being able to use the helpers in drm and avoiding an
> extra layer of translation seems like the better option to me.  So my
> vote would be drivers/gpu/cdf.

Well, we need to think about that. I would like to keep CDF independent
of DRM. I don't like tying different components/frameworks together if
there's no real need for that.

Also, something that Laurent mentioned in our face-to-face discussions:
Some IPs/chips can be used for other purposes than with DRM.

He had an example of a board, that (if I understood right) gets video
signal from somewhere outside the board, processes the signal with some
IPs/chips, and then outputs the signal. So there's no framebuffer, and
the image is not stored anywhere. I think the framework used in these
cases is always v4l2.

The IPs/chips in the above model may be the exact same IPs/chips that
are used with "normal" display. If the CDF was tied to DRM, using the
same drivers for normal and these streaming cases would probably not be
possible.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: [RFC v2 0/5] Common Display Framework
From: Rob Clark @ 2012-12-19 15:26 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Laurent Pinchart, Tomi Valkeinen, Thomas Petazzoni,
	Linux Fbdev development list, Philipp Zabel, Tom Gall,
	Ragesh Radhakrishnan, dri-devel@lists.freedesktop.org,
	Kyungmin Park, Benjamin Gaignard, Bryan Wu, Maxime Ripard,
	Vikas Sajjan, Sumit Semwal, Sebastien Guiriec,
	linux-media@vger.kernel.org
In-Reply-To: <87pq26ay2z.fsf@intel.com>

On Wed, Dec 19, 2012 at 8:57 AM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
>
> Hi Laurent -
>
> On Tue, 18 Dec 2012, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>> Hi Jani,
>>
>> On Monday 17 December 2012 18:53:37 Jani Nikula wrote:
>>> I can see the need for a framework for DSI panels and such (in fact Tomi
>>> and I have talked about it like 2-3 years ago already!) but what is the
>>> story for HDMI and DP? In particular, what's the relationship between
>>> DRM and CDF here? Is there a world domination plan to switch the DRM
>>> drivers to use this framework too? ;) Do you have some rough plans how
>>> DRM and CDF should work together in general?
>>
>> There's always a world domination plan, isn't there ? :-)
>>
>> I certainly want CDF to be used by DRM (or more accurately KMS). That's what
>> the C stands for, common refers to sharing panel and other display entity
>> drivers between FBDEV, KMS and V4L2.
>>
>> I currently have no plan to expose CDF internals to userspace through the KMS
>> API. We might have to do so later if the hardware complexity grows in such a
>> way that finer control than what KMS provides needs to be exposed to
>> userspace, but I don't think we're there yet. The CDF API will thus only be
>> used internally in the kernel by display controller drivers. The KMS core
>> might get functions to handle common display entity operations, but the bulk
>> of the work will be in the display controller drivers to start with. We will
>> then see what can be abstracted in KMS helper functions.
>>
>> Regarding HDMI and DP, I imagine HDMI and DP drivers that would use the CDF
>> API. That's just a thought for now, I haven't tried to implement them, but it
>> would be nice to handle HDMI screens and DPI/DBI/DSI panels in a generic way.
>>
>> Do you have thoughts to share on this topic ?
>
> It just seems to me that, at least from a DRM/KMS perspective, adding
> another layer (ÍF) for HDMI or DP (or legacy outputs) would be
> overengineering it. They are pretty well standardized, and I don't see
> there would be a need to write multiple display drivers for them. Each
> display controller has one, and can easily handle any chip specific
> requirements right there. It's my gut feeling that an additional
> framework would just get in the way. Perhaps there could be more common
> HDMI/DP helper style code in DRM to reduce overlap across KMS drivers,
> but that's another thing.
>
> So is the HDMI/DP drivers using CDF a more interesting idea from a
> non-DRM perspective? Or, put another way, is it more of an alternative
> to using DRM? Please enlighten me if there's some real benefit here that
> I fail to see!

fwiw, I think there are at least a couple cases where multiple SoC's
have the same HDMI IP block.

And, there are also external HDMI encoders (for example connected over
i2c) that can also be shared between boards.  So I think there will be
a number of cases where CDF is appropriate for HDMI drivers.  Although
trying to keep this all independent of DRM (as opposed to just
something similar to what drivers/gpu/i2c is today) seems a bit
overkill for me.  Being able to use the helpers in drm and avoiding an
extra layer of translation seems like the better option to me.  So my
vote would be drivers/gpu/cdf.

BR,
-R

> For DSI panels (or DSI-to-whatever bridges) it's of course another
> story. You typically need a panel specific driver. And here I see the
> main point of the whole CDF: decoupling display controllers and the
> panel drivers, and sharing panel (and converter chip) specific drivers
> across display controllers. Making it easy to write new drivers, as
> there would be a model to follow. I'm definitely in favour of coming up
> with some framework that would tackle that.
>
>
> BR,
> Jani.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 0/5] Common Display Framework
From: Tomi Valkeinen @ 2012-12-19 15:07 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Laurent Pinchart, Thomas Petazzoni, linux-fbdev, Philipp Zabel,
	Tom Gall, Ragesh Radhakrishnan, dri-devel, Rob Clark,
	Kyungmin Park, Benjamin Gaignard, Bryan Wu, Maxime Ripard,
	Vikas Sajjan, Sumit Semwal, Sebastien Guiriec, linux-media
In-Reply-To: <87pq26ay2z.fsf@intel.com>

[-- Attachment #1: Type: text/plain, Size: 2265 bytes --]

On 2012-12-19 16:57, Jani Nikula wrote:

> It just seems to me that, at least from a DRM/KMS perspective, adding
> another layer (=CDF) for HDMI or DP (or legacy outputs) would be
> overengineering it. They are pretty well standardized, and I don't see
> there would be a need to write multiple display drivers for them. Each
> display controller has one, and can easily handle any chip specific
> requirements right there. It's my gut feeling that an additional
> framework would just get in the way. Perhaps there could be more common
> HDMI/DP helper style code in DRM to reduce overlap across KMS drivers,
> but that's another thing.
> 
> So is the HDMI/DP drivers using CDF a more interesting idea from a
> non-DRM perspective? Or, put another way, is it more of an alternative
> to using DRM? Please enlighten me if there's some real benefit here that
> I fail to see!

The use of CDF is an option, not something that has to be done. A DRM
driver developer may use it if it gives benefit for him for that
particular driver.

I don't know much about desktop display hardware, but I guess that using
CDF would not really give much there. In some cases it could, if the IPs
used on the graphics card are something that are used elsewhere also
(sounds quite unlikely, though). In that case there could be separate
drivers for the IPs.

And note that CDF is not really about the dispc side, i.e. the part that
creates the video stream from pixels in the memory. It's more about the
components after that, and how to connect those components.

> For DSI panels (or DSI-to-whatever bridges) it's of course another
> story. You typically need a panel specific driver. And here I see the
> main point of the whole CDF: decoupling display controllers and the
> panel drivers, and sharing panel (and converter chip) specific drivers
> across display controllers. Making it easy to write new drivers, as
> there would be a model to follow. I'm definitely in favour of coming up
> with some framework that would tackle that.

Right. But if you implement drivers for DSI panels with CDF for, say,
OMAP, I think it's simpler to use CDF also for HDMI/DP on OMAP.
Otherwise it'll be a mishmash with two different models.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: [RFC v2 0/5] Common Display Framework
From: Jani Nikula @ 2012-12-19 14:57 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tomi Valkeinen, Thomas Petazzoni, linux-fbdev, Philipp Zabel,
	Tom Gall, Ragesh Radhakrishnan, dri-devel, Rob Clark,
	Kyungmin Park, Benjamin Gaignard, Bryan Wu, Maxime Ripard,
	Vikas Sajjan, Sumit Semwal, Sebastien Guiriec, linux-media
In-Reply-To: <1671267.x0lxGrFjjV@avalon>


Hi Laurent -

On Tue, 18 Dec 2012, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> Hi Jani,
>
> On Monday 17 December 2012 18:53:37 Jani Nikula wrote:
>> I can see the need for a framework for DSI panels and such (in fact Tomi
>> and I have talked about it like 2-3 years ago already!) but what is the
>> story for HDMI and DP? In particular, what's the relationship between
>> DRM and CDF here? Is there a world domination plan to switch the DRM
>> drivers to use this framework too? ;) Do you have some rough plans how
>> DRM and CDF should work together in general?
>
> There's always a world domination plan, isn't there ? :-)
>
> I certainly want CDF to be used by DRM (or more accurately KMS). That's what 
> the C stands for, common refers to sharing panel and other display entity 
> drivers between FBDEV, KMS and V4L2.
>
> I currently have no plan to expose CDF internals to userspace through the KMS 
> API. We might have to do so later if the hardware complexity grows in such a 
> way that finer control than what KMS provides needs to be exposed to 
> userspace, but I don't think we're there yet. The CDF API will thus only be 
> used internally in the kernel by display controller drivers. The KMS core 
> might get functions to handle common display entity operations, but the bulk 
> of the work will be in the display controller drivers to start with. We will 
> then see what can be abstracted in KMS helper functions.
>
> Regarding HDMI and DP, I imagine HDMI and DP drivers that would use the CDF 
> API. That's just a thought for now, I haven't tried to implement them, but it 
> would be nice to handle HDMI screens and DPI/DBI/DSI panels in a generic way.
>
> Do you have thoughts to share on this topic ?

It just seems to me that, at least from a DRM/KMS perspective, adding
another layer (ÍF) for HDMI or DP (or legacy outputs) would be
overengineering it. They are pretty well standardized, and I don't see
there would be a need to write multiple display drivers for them. Each
display controller has one, and can easily handle any chip specific
requirements right there. It's my gut feeling that an additional
framework would just get in the way. Perhaps there could be more common
HDMI/DP helper style code in DRM to reduce overlap across KMS drivers,
but that's another thing.

So is the HDMI/DP drivers using CDF a more interesting idea from a
non-DRM perspective? Or, put another way, is it more of an alternative
to using DRM? Please enlighten me if there's some real benefit here that
I fail to see!

For DSI panels (or DSI-to-whatever bridges) it's of course another
story. You typically need a panel specific driver. And here I see the
main point of the whole CDF: decoupling display controllers and the
panel drivers, and sharing panel (and converter chip) specific drivers
across display controllers. Making it easy to write new drivers, as
there would be a model to follow. I'm definitely in favour of coming up
with some framework that would tackle that.


BR,
Jani.

^ permalink raw reply

* Re: [RFC 0/6] Common Display Framework-T
From: Tomi Valkeinen @ 2012-12-19 14:53 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linux-fbdev, dri-devel
In-Reply-To: <5024715.KeGkjOKZ3Q@avalon>

[-- Attachment #1: Type: text/plain, Size: 14789 bytes --]

On 2012-12-19 15:21, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Friday 14 December 2012 16:27:26 Tomi Valkeinen wrote:
>> Hi,
>>
>> I have been testing Common Display Framework on OMAP, and making changes
>> that I've discussed in the posts I've sent in reply to the CDF series from
>> Laurent. While my CDF code is rather hacky and not at all ready, I wanted
>> to post the code for comments and also as a reference code to my posts.
>>
>> So here is CDF-T (Tomi-edition =).
> 
> We've discussed your approach extensively face-to-face today so I won't review 
> the patches in detail, but I will instead summarize our discussion to make 
> sure we understood each other (and let other developers jump in).

I have some comments =). But mostly it looks good.

> For the purpose of this discussion the term "display controller driver" (or 
> just "display controller") refer to both the low-level driver layer that 
> communicates directly with the display controller hardware, and to the higher-
> level driver layer that implements and exposes the userspace API (FBDEV, KMS 
> and/or V4L). Those layers can be implemented in multiple kernel modules (such 
> as in the OMAP DSS case, with omapdss for the low-level layer and omapdrm, 
> omapfb and omapvout for the API-level layer) or a single kernel module.
> 
> Control model
> -------------
> 
> The figure at http://www.ideasonboard.org/media/cdf/cdf-panel-control-
> model.png shows the CDF control model.
> 
> The panel object depicted on the figure doesn't need to be a panel in the 
> stricter sense but could be any chain of off-SoC (both on-board or off-board) 
> display entities. It however helps thinking about it as a panel and doesn't 
> hurt the model.

I don't think it needs to be off-soc. The dispc and the panel in the
image could be any two components, in- or off-soc.

> The panel is controlled through abstract control requests. Those requests are 
> used to retrieve panel information (such as the physical size, the supported 
> video modes, EDID information, ...), set the panel configuration (such as the 
> active video timings) or control the panel operation state (enabling/disabling 
> the panel, controlling panel blanking and power management, ...). They are 
> exposed by the panel using function pointers, and called by other kernel 
> components in response to userspace requests (through the FBDEV, KMS or V4L2 
> APIs) or in-kernel events (for instance hotplug notifications).
> 
> In response to the control requests the panel driver will communicate with the 
> panel through the panel control bus (I2C, SPI, DBI, DSI, GPIO, ..., not shown 
> on the figure) and will control the video stream it receives on its input.
> 
> The panel is connected at the hardware level to a video source (shown as a 
> green hashed rectangle) that provides it with a video stream. The video stream 
> flows from the video source to the panel and is directly controlled by its 
> source, as shown by the green arrow from the display controller to the video 
> stream. The video source exposes stream control operations as function 
> pointers that are used by the panel to control the video stream, as shown by 
> the green arrow from the panel to the video source.
> 
> The figure at http://www.ideasonboard.org/media/cdf/cdf-panel-control-
> model-2.png shows the call flow across entities when the panel is a pipeline 
> made of more than a single entity. In this case the SoC (on the left of the 
> dashed line) outputs a video stream on a DSI bus connected to a DSI to LVDS 
> transmitter. The output of the DSI to LVDS transmitter is connected to an LVDS 
> panel (or, more accurately, an LVDS panel module made of an LVDS panel 
> controller and a panel).

Here also I don't see any reason to separate in- or off-soc components.
I think the call from DISPC, which now goes to the transmitter, should
first go to the DPI/DSI block. Whether the DPI/DSI block is in- or
off-soc should be irrelevant regarding CDF.

> The transmitter and panel module are seen by the display controller and 
> userspace API implementations as a single entity that exposes control request 
> operations and controls its input video stream. When a control request is 

I don't like the sound of this. I think the CDF shouldn't care how the
userspace API is implemented. There's no reason in CDF level to separate
in- or out-soc entities, nor expose only one entity. If DRM requires
mapping DRM's crtc, encoder and connector to, respectively, dispc,
DPI/DSI, the-rest, it should be able to do that, but CDF shouldn't force
that model.

Of course, the implementor of the particular SoC display driver could
decide to use one display entity that covers both dispc and DPI/DSI
blocks. But at least I would like to have OMAP's DISPC as a display
entity (or actually multiple entities, one for each DISPC output), and
the various in-SoC DPI-to-something encoders as display entities.

> performed (outermost green arrow) the DSI to LVDS transmitter will propagate 
> it to the panel, possibly mangling the input parameters or the response. For 
> panel operation state control requests the last entity in the pipeline will 
> likely want to control the video stream it receives on its input. The video 
> stream control calls will be propagated from right to left as shown by the red 
> arrows.
> 
> Every entity in the call stack can communicate with its hardware device 
> through the corresponding control bus, and/or control the video stream it 
> receives on its input.
> 
> This model allows filtering out modes and timings supported by the panel but 
> unsupported by the transmitter and mangling the modes and timings according to 
> the transmitter limitations. It has no complexity drawback for simple devices, 
> as the corresponding drivers can just forward the calls directly. Similar use 
> cases could exist for other control operations than mode and information 
> retrieval.
> 
> Discovery
> ---------
> 
> Before being able to issue control requests, panel devices need to be 
> discovered and associated with the connected display controller(s).
> 
> Panels and display controllers are cross-dependent. There is no way around 

Perhaps semantics, but I don't think they are cross-dependent. True,
they will call ops in each other, but the dispc will get the pointer to
the panel when the panel connects the dispc and the panel. And when the
panel disconnects, dispc will lose the reference to the panel.

For me, cross-dependent would mean that dispc could have a reference to
the panel regardless of what the panel does. In our case it is not so,
and there's no harm with the dispc's reference to the panel, as the
panel can remove it (almost) at any time.

> that, as the display controller needs a reference to the panel to call control 
> requests in response to userspace API, and the panel needs a reference to the 
> display controller to call video stream control functions (in addition to 
> requiring generic resources such as clocks, GPIOs or even regulators that 
> could be provided by the display controller).
> 
> As we can't probe the display controller and the panel together, a probe order 
> needs to be defined. The decision was to consider video sources as resources 
> and defer panel probing until all required resources (video stream source, 
> clocks, GPIOs, regulators and more) are available. Display controller probing 
> must succeed without the panel being available. This mimicks the hotpluggable 
> monitor model (VGA, HDMI, DP) that doesn't prevent display controllers from 
> being successfully probed without a connected monitor.
> 
> Our design goal is to handle panel discovery in a similar (if not identical) 
> way as HDMI/DP hotplug in order to implement a single display discovery method 
> in display controller drivers. This might not be achievable, in which case 
> we'll reconsider the design requirement.
> 
> When the display controller driver probes the device it will register the 
> video source(s) at the output of the display controller with the CDF core. 
> Those sources will be identified by the display controller dev_name() and a 
> source integer index. A new structure, likely called display_entity_port, will 
> be used to represent a source or sink video port on a display entity.
> 
> Panel drivers will handle video sources as resources. They will retrieve at 
> probe time the video source the panel is connected to using a phandle or a 
> source name (depending on whether the platform uses DT). If the source isn't 
> available the probe function will return -EPROBE_DEFER.
> 
> In addition to the video stream control operations mentioned above, ports will 
> also expose a connect/disconnect operation use to notify them of 
> connection/disconnection events. After retrieving the connected video source 
> panel drivers call the connect/disconnect operation on the video source to 
> notify it that the panel is available.
> 
> When the panel is a pipeline made of more than a single entity, entities are 
> probed in video source to video sink order. Out-of-order probe will result in 
> probe deferral as explained above due to the video source not being available, 
> resulting in the source to sink probe order. Entities should not call the 
> connect operation of their video source at probe time in that case, but only 
> when their own connect operation for the video source(s) they provide to the 
> next entity is called by the next entity. Connect operations will thus be 
> called in sink to source order starting at the entity at the end of the 
> pipeline and going all the way back to the display controller.
> 
> This notification system is a hotplug mechanism that replaces the display 
> entity notifier system from my previous RFC. Alan Cox rightly objected to the 
> notification system, arguing that such system-wide notifications were used by 
> FBDEV and very subject to abuse. I agree with his argument, this new mechanism 
> should result in a cleaner implementation as video sources will only be 
> notified of connect/disconnect events for the entity they're connected to.
> 
> DBI/DSI busses
> --------------
> 
> My RFC introduced a DBI bus using the Linux device and bus model. Its purpose 
> was multifold:
> 
> - Support (un)registration, matching and binding of devices and drivers.
> 
> - Provide power management (suspend/resume) services through the standard 
> Linux PM bus/device model, to make sure that DBI devices will be 
> suspended/resumed after/before their DBI bus controller.
> 
> - Provide bus services to access the connected devices. For DBI that took the 
> form of command read and data read/write functions.
> 
> A DSI bus implementation using the same model was also planned.
> 
> Tomi's patches removed the DBI bus and replaced DBI devices with platform 
> devices, moving the bus services implementation to the video source. DBI and 
> DSI busses are always either pure video or video + control busses (although 
> controlling a DPI panel through DSI is conceivable, nobody in his right mind, 
> not even a hardware engineer, would likely implement that), so there will 
> always be a video source to provide the DBI/DSI control operations.
> 
> (Un)registration, matching and binding of devices and drivers is provided by 
> the platform device bus. Bus services to access connected devices are provided 
> by the video source, wrapper functions will be used to handle serialization 
> and locking, and possibly to offer higher level services (such as DCS for 
> instance).
> 
> One drawback of using the platform bus is that PM relationships between the 
> bus master and slaves will not be taken into account during suspend/resume. 
> However, a similar issue exists for DPI panels, and PM relationships at the 
> video bus level for DBI and DSI are not handled by the DBI/DSI busses either. 
> As we need a generic solution to handle those (likely through early suspend 
> and late resume), the same solution can be used to handle DBI and DSI control 
> bus PM relationships without requiring a Linux DBI or DSI bus.
> 
> Even though I still like the idea of DBI and DSI busses, I agree with Tomi 
> that they're not strictly needed and I will drop them.

I'd like to highlight two points I made about the bus model:

- If DBI is used only for video, there's no DBI bus. How to configure
DBI in this case?

- If DBI is used for control and video, we have two separate APIs for
the bus. In theory it's possible to handle this, but in practice it may
be impossible, especially for more complex busses like DSI.

I think both of those issues would make the bus model very difficult to
implement. I have no idea how it could be done neatly. So as I see it,
it's not only about "not strictly needed", but that the bus model
wouldn't work without complex code.

> Entity model
> ------------
> 
> Tomi's proposal split the display entities into video sources (struct  
> video_source) and display entities (struct display_entity). To make generic 
> pipeline operations easier, we agreed to merge the video source and the 
> display entity back. struct display_entity thus models a display entity that 
> has any number of sink and/or source ports, modeled as struct 
> display_entity_port instances.
> 
> Video stream operations will be exposed by the display entity as function 
> pointers and will take a port reference as argument (this could take the form 
> of struct display_entity * and port index, or struct display_entity_port *). 

I'd very much like to have only one parameter to pass, as there may be
lots of ops for some busses. Having two parameters to refer to the
source is just extra code that has no extra benefit when using video
source ops. Then again, having separate port index parameter could be
perhaps simpler to implement for the one handling the video source ops,
so...

> The DVI and DSI operations model proposed by Tomi in this patch series will be 
> kept.
> 
> Points that we forgot to discuss
> --------------------------------
> 
> - DISPLAY_ENTITY_STREAM_SINGLE_SHOT vs. update() operation

Ah, yes, we missed that. I think it's possible to use SINGLE_SHOT, but
then it requires some kind of system to inform about finished update. Or
we could, of course, decide that informing about the update is done in
dispc-specific code, like handling VSYNC.

Hmm, except that won't probably work, as the panel (or any DSI device)
may need to know if the DSI bus is currently used or not.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: [RFC 0/6] Common Display Framework-T
From: Laurent Pinchart @ 2012-12-19 13:21 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-fbdev, dri-devel
In-Reply-To: <1355495252-26364-1-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

On Friday 14 December 2012 16:27:26 Tomi Valkeinen wrote:
> Hi,
> 
> I have been testing Common Display Framework on OMAP, and making changes
> that I've discussed in the posts I've sent in reply to the CDF series from
> Laurent. While my CDF code is rather hacky and not at all ready, I wanted
> to post the code for comments and also as a reference code to my posts.
> 
> So here is CDF-T (Tomi-edition =).

We've discussed your approach extensively face-to-face today so I won't review 
the patches in detail, but I will instead summarize our discussion to make 
sure we understood each other (and let other developers jump in).

For the purpose of this discussion the term "display controller driver" (or 
just "display controller") refer to both the low-level driver layer that 
communicates directly with the display controller hardware, and to the higher-
level driver layer that implements and exposes the userspace API (FBDEV, KMS 
and/or V4L). Those layers can be implemented in multiple kernel modules (such 
as in the OMAP DSS case, with omapdss for the low-level layer and omapdrm, 
omapfb and omapvout for the API-level layer) or a single kernel module.

Control model
-------------

The figure at http://www.ideasonboard.org/media/cdf/cdf-panel-control-
model.png shows the CDF control model.

The panel object depicted on the figure doesn't need to be a panel in the 
stricter sense but could be any chain of off-SoC (both on-board or off-board) 
display entities. It however helps thinking about it as a panel and doesn't 
hurt the model.

The panel is controlled through abstract control requests. Those requests are 
used to retrieve panel information (such as the physical size, the supported 
video modes, EDID information, ...), set the panel configuration (such as the 
active video timings) or control the panel operation state (enabling/disabling 
the panel, controlling panel blanking and power management, ...). They are 
exposed by the panel using function pointers, and called by other kernel 
components in response to userspace requests (through the FBDEV, KMS or V4L2 
APIs) or in-kernel events (for instance hotplug notifications).

In response to the control requests the panel driver will communicate with the 
panel through the panel control bus (I2C, SPI, DBI, DSI, GPIO, ..., not shown 
on the figure) and will control the video stream it receives on its input.

The panel is connected at the hardware level to a video source (shown as a 
green hashed rectangle) that provides it with a video stream. The video stream 
flows from the video source to the panel and is directly controlled by its 
source, as shown by the green arrow from the display controller to the video 
stream. The video source exposes stream control operations as function 
pointers that are used by the panel to control the video stream, as shown by 
the green arrow from the panel to the video source.

The figure at http://www.ideasonboard.org/media/cdf/cdf-panel-control-
model-2.png shows the call flow across entities when the panel is a pipeline 
made of more than a single entity. In this case the SoC (on the left of the 
dashed line) outputs a video stream on a DSI bus connected to a DSI to LVDS 
transmitter. The output of the DSI to LVDS transmitter is connected to an LVDS 
panel (or, more accurately, an LVDS panel module made of an LVDS panel 
controller and a panel).

The transmitter and panel module are seen by the display controller and 
userspace API implementations as a single entity that exposes control request 
operations and controls its input video stream. When a control request is 
performed (outermost green arrow) the DSI to LVDS transmitter will propagate 
it to the panel, possibly mangling the input parameters or the response. For 
panel operation state control requests the last entity in the pipeline will 
likely want to control the video stream it receives on its input. The video 
stream control calls will be propagated from right to left as shown by the red 
arrows.

Every entity in the call stack can communicate with its hardware device 
through the corresponding control bus, and/or control the video stream it 
receives on its input.

This model allows filtering out modes and timings supported by the panel but 
unsupported by the transmitter and mangling the modes and timings according to 
the transmitter limitations. It has no complexity drawback for simple devices, 
as the corresponding drivers can just forward the calls directly. Similar use 
cases could exist for other control operations than mode and information 
retrieval.

Discovery
---------

Before being able to issue control requests, panel devices need to be 
discovered and associated with the connected display controller(s).

Panels and display controllers are cross-dependent. There is no way around 
that, as the display controller needs a reference to the panel to call control 
requests in response to userspace API, and the panel needs a reference to the 
display controller to call video stream control functions (in addition to 
requiring generic resources such as clocks, GPIOs or even regulators that 
could be provided by the display controller).

As we can't probe the display controller and the panel together, a probe order 
needs to be defined. The decision was to consider video sources as resources 
and defer panel probing until all required resources (video stream source, 
clocks, GPIOs, regulators and more) are available. Display controller probing 
must succeed without the panel being available. This mimicks the hotpluggable 
monitor model (VGA, HDMI, DP) that doesn't prevent display controllers from 
being successfully probed without a connected monitor.

Our design goal is to handle panel discovery in a similar (if not identical) 
way as HDMI/DP hotplug in order to implement a single display discovery method 
in display controller drivers. This might not be achievable, in which case 
we'll reconsider the design requirement.

When the display controller driver probes the device it will register the 
video source(s) at the output of the display controller with the CDF core. 
Those sources will be identified by the display controller dev_name() and a 
source integer index. A new structure, likely called display_entity_port, will 
be used to represent a source or sink video port on a display entity.

Panel drivers will handle video sources as resources. They will retrieve at 
probe time the video source the panel is connected to using a phandle or a 
source name (depending on whether the platform uses DT). If the source isn't 
available the probe function will return -EPROBE_DEFER.

In addition to the video stream control operations mentioned above, ports will 
also expose a connect/disconnect operation use to notify them of 
connection/disconnection events. After retrieving the connected video source 
panel drivers call the connect/disconnect operation on the video source to 
notify it that the panel is available.

When the panel is a pipeline made of more than a single entity, entities are 
probed in video source to video sink order. Out-of-order probe will result in 
probe deferral as explained above due to the video source not being available, 
resulting in the source to sink probe order. Entities should not call the 
connect operation of their video source at probe time in that case, but only 
when their own connect operation for the video source(s) they provide to the 
next entity is called by the next entity. Connect operations will thus be 
called in sink to source order starting at the entity at the end of the 
pipeline and going all the way back to the display controller.

This notification system is a hotplug mechanism that replaces the display 
entity notifier system from my previous RFC. Alan Cox rightly objected to the 
notification system, arguing that such system-wide notifications were used by 
FBDEV and very subject to abuse. I agree with his argument, this new mechanism 
should result in a cleaner implementation as video sources will only be 
notified of connect/disconnect events for the entity they're connected to.

DBI/DSI busses
--------------

My RFC introduced a DBI bus using the Linux device and bus model. Its purpose 
was multifold:

- Support (un)registration, matching and binding of devices and drivers.

- Provide power management (suspend/resume) services through the standard 
Linux PM bus/device model, to make sure that DBI devices will be 
suspended/resumed after/before their DBI bus controller.

- Provide bus services to access the connected devices. For DBI that took the 
form of command read and data read/write functions.

A DSI bus implementation using the same model was also planned.

Tomi's patches removed the DBI bus and replaced DBI devices with platform 
devices, moving the bus services implementation to the video source. DBI and 
DSI busses are always either pure video or video + control busses (although 
controlling a DPI panel through DSI is conceivable, nobody in his right mind, 
not even a hardware engineer, would likely implement that), so there will 
always be a video source to provide the DBI/DSI control operations.

(Un)registration, matching and binding of devices and drivers is provided by 
the platform device bus. Bus services to access connected devices are provided 
by the video source, wrapper functions will be used to handle serialization 
and locking, and possibly to offer higher level services (such as DCS for 
instance).

One drawback of using the platform bus is that PM relationships between the 
bus master and slaves will not be taken into account during suspend/resume. 
However, a similar issue exists for DPI panels, and PM relationships at the 
video bus level for DBI and DSI are not handled by the DBI/DSI busses either. 
As we need a generic solution to handle those (likely through early suspend 
and late resume), the same solution can be used to handle DBI and DSI control 
bus PM relationships without requiring a Linux DBI or DSI bus.

Even though I still like the idea of DBI and DSI busses, I agree with Tomi 
that they're not strictly needed and I will drop them.

Entity model
------------

Tomi's proposal split the display entities into video sources (struct  
video_source) and display entities (struct display_entity). To make generic 
pipeline operations easier, we agreed to merge the video source and the 
display entity back. struct display_entity thus models a display entity that 
has any number of sink and/or source ports, modeled as struct 
display_entity_port instances.

Video stream operations will be exposed by the display entity as function 
pointers and will take a port reference as argument (this could take the form 
of struct display_entity * and port index, or struct display_entity_port *). 
The DVI and DSI operations model proposed by Tomi in this patch series will be 
kept.

Points that we forgot to discuss
--------------------------------

- DISPLAY_ENTITY_STREAM_SINGLE_SHOT vs. update() operation

I'll look into that.

Please let me know if I've forgotten anything.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH V2 2/6] OMAPDSS: DISPC: Move DISPC specific dss_reg_fields to dispc_features
From: Chandrabhanu Mahapatra @ 2012-12-19  9:39 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <50CF1202.70507@ti.com>

On Monday 17 December 2012 06:07 PM, Tomi Valkeinen wrote:
> On 2012-12-05 12:16, Chandrabhanu Mahapatra wrote:
>> The register fields in dss_reg_fields specific to DISPC are moved from struct
>> omap_dss_features to corresponding dispc_reg_fields, initialized in struct
>> dispc_features, thereby enabling local access.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  drivers/video/omap2/dss/dispc.c        |  114 ++++++++++++++++++++++++--------
>>  drivers/video/omap2/dss/dss.h          |    4 ++
>>  drivers/video/omap2/dss/dss_features.c |   28 --------
>>  drivers/video/omap2/dss/dss_features.h |    7 --
>>  4 files changed, 89 insertions(+), 64 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index bbba83f..ee4b152 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -80,6 +80,16 @@ struct dispc_irq_stats {
>>  	unsigned irqs[32];
>>  };
>>  
>> +struct dispc_reg_fields {
>> +	struct omapdss_reg_field firhinc;
>> +	struct omapdss_reg_field firvinc;
>> +	struct omapdss_reg_field fifo_low_thresh;
>> +	struct omapdss_reg_field fifo_high_thresh;
>> +	struct omapdss_reg_field fifosize;
>> +	struct omapdss_reg_field hori_accu;
>> +	struct omapdss_reg_field vert_accu;
>> +};
>> +
>>  struct dispc_features {
>>  	u8 sw_start;
>>  	u8 fp_start;
>> @@ -110,6 +120,8 @@ struct dispc_features {
>>  
>>  	u32 buffer_size_unit; /* in bytes */
>>  	u32 burst_size_unit; /* in bytes */
>> +
>> +	struct dispc_reg_fields *reg_fields;
> 
> This can be pointer to const.
> 

Yes, the same thing can also be done in other dispc and dsi reg_fields
and param_ranges.

>>  };
>>  
>>  #define DISPC_MAX_NR_FIFOS 5
>> @@ -1137,17 +1149,17 @@ static void dispc_mgr_set_size(enum omap_channel channel, u16 width,
>>  
>>  static void dispc_init_fifos(void)
>>  {
>> -	u32 size;
>> +	u32 size, unit;
>>  	int fifo;
>> -	u8 start, end;
>> -	u32 unit;
>> +	const struct omapdss_reg_field *fifo_field;
>>  
>>  	unit = dispc.feat->buffer_size_unit;
>>  
>> -	dss_feat_get_reg_field(FEAT_REG_FIFOSIZE, &start, &end);
>> +	fifo_field = &dispc.feat->reg_fields->fifosize;
>>  
>>  	for (fifo = 0; fifo < dispc.feat->num_fifos; ++fifo) {
>> -		size = REG_GET(DISPC_OVL_FIFO_SIZE_STATUS(fifo), start, end);
>> +		size = REG_GET(DISPC_OVL_FIFO_SIZE_STATUS(fifo),
>> +					fifo_field->start, fifo_field->end);
>>  		size *= unit;
>>  		dispc.fifo_size[fifo] = size;
>>  
>> @@ -1197,8 +1209,8 @@ static u32 dispc_ovl_get_fifo_size(enum omap_plane plane)
>>  
>>  void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high)
>>  {
>> -	u8 hi_start, hi_end, lo_start, lo_end;
>>  	u32 unit;
>> +	const struct omapdss_reg_field *hi_field, *lo_field;
>>  
>>  	unit = dispc.feat->buffer_size_unit;
>>  
>> @@ -1208,20 +1220,20 @@ void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high)
>>  	low /= unit;
>>  	high /= unit;
>>  
>> -	dss_feat_get_reg_field(FEAT_REG_FIFOHIGHTHRESHOLD, &hi_start, &hi_end);
>> -	dss_feat_get_reg_field(FEAT_REG_FIFOLOWTHRESHOLD, &lo_start, &lo_end);
>> +	hi_field = &dispc.feat->reg_fields->fifo_high_thresh;
>> +	lo_field = &dispc.feat->reg_fields->fifo_low_thresh;
>>  
>>  	DSSDBG("fifo(%d) threshold (bytes), old %u/%u, new %u/%u\n",
>>  			plane,
>>  			REG_GET(DISPC_OVL_FIFO_THRESHOLD(plane),
>> -				lo_start, lo_end) * unit,
>> +				lo_field->start, lo_field->end) * unit,
>>  			REG_GET(DISPC_OVL_FIFO_THRESHOLD(plane),
>> -				hi_start, hi_end) * unit,
>> +				hi_field->start, hi_field->end) * unit,
>>  			low * unit, high * unit);
>>  
>>  	dispc_write_reg(DISPC_OVL_FIFO_THRESHOLD(plane),
>> -			FLD_VAL(high, hi_start, hi_end) |
>> -			FLD_VAL(low, lo_start, lo_end));
>> +			FLD_VAL(high, hi_field->start, hi_field->end) |
>> +			FLD_VAL(low, lo_field->start, lo_field->end));
>>  }
>>  
>>  void dispc_enable_fifomerge(bool enable)
>> @@ -1289,14 +1301,13 @@ static void dispc_ovl_set_fir(enum omap_plane plane,
>>  	u32 val;
>>  
>>  	if (color_comp = DISPC_COLOR_COMPONENT_RGB_Y) {
>> -		u8 hinc_start, hinc_end, vinc_start, vinc_end;
>> +		const struct omapdss_reg_field *hinc_field, *vinc_field;
>>  
>> -		dss_feat_get_reg_field(FEAT_REG_FIRHINC,
>> -					&hinc_start, &hinc_end);
>> -		dss_feat_get_reg_field(FEAT_REG_FIRVINC,
>> -					&vinc_start, &vinc_end);
>> -		val = FLD_VAL(vinc, vinc_start, vinc_end) |
>> -				FLD_VAL(hinc, hinc_start, hinc_end);
>> +		hinc_field = &dispc.feat->reg_fields->firhinc;
>> +		vinc_field = &dispc.feat->reg_fields->firvinc;
>> +
>> +		val = FLD_VAL(vinc, vinc_field->start, vinc_field->end) |
>> +			FLD_VAL(hinc, hinc_field->start, hinc_field->end);
>>  
>>  		dispc_write_reg(DISPC_OVL_FIR(plane), val);
>>  	} else {
>> @@ -1308,13 +1319,13 @@ static void dispc_ovl_set_fir(enum omap_plane plane,
>>  static void dispc_ovl_set_vid_accu0(enum omap_plane plane, int haccu, int vaccu)
>>  {
>>  	u32 val;
>> -	u8 hor_start, hor_end, vert_start, vert_end;
>> +	const struct omapdss_reg_field *haccu_field, *vaccu_field;
>>  
>> -	dss_feat_get_reg_field(FEAT_REG_HORIZONTALACCU, &hor_start, &hor_end);
>> -	dss_feat_get_reg_field(FEAT_REG_VERTICALACCU, &vert_start, &vert_end);
>> +	haccu_field = &dispc.feat->reg_fields->hori_accu;
>> +	vaccu_field = &dispc.feat->reg_fields->vert_accu;
>>  
>> -	val = FLD_VAL(vaccu, vert_start, vert_end) |
>> -			FLD_VAL(haccu, hor_start, hor_end);
>> +	val = FLD_VAL(vaccu, vaccu_field->start, vaccu_field->end) |
>> +		FLD_VAL(haccu, haccu_field->start, haccu_field->end);
>>  
>>  	dispc_write_reg(DISPC_OVL_ACCU0(plane), val);
>>  }
>> @@ -1322,13 +1333,13 @@ static void dispc_ovl_set_vid_accu0(enum omap_plane plane, int haccu, int vaccu)
>>  static void dispc_ovl_set_vid_accu1(enum omap_plane plane, int haccu, int vaccu)
>>  {
>>  	u32 val;
>> -	u8 hor_start, hor_end, vert_start, vert_end;
>> +	const struct omapdss_reg_field *haccu_field, *vaccu_field;
>>  
>> -	dss_feat_get_reg_field(FEAT_REG_HORIZONTALACCU, &hor_start, &hor_end);
>> -	dss_feat_get_reg_field(FEAT_REG_VERTICALACCU, &vert_start, &vert_end);
>> +	haccu_field = &dispc.feat->reg_fields->hori_accu;
>> +	vaccu_field = &dispc.feat->reg_fields->vert_accu;
>>  
>> -	val = FLD_VAL(vaccu, vert_start, vert_end) |
>> -			FLD_VAL(haccu, hor_start, hor_end);
>> +	val = FLD_VAL(vaccu, vaccu_field->start, vaccu_field->end) |
>> +		FLD_VAL(haccu, haccu_field->start, haccu_field->end);
>>  
>>  	dispc_write_reg(DISPC_OVL_ACCU1(plane), val);
>>  }
>> @@ -4048,6 +4059,46 @@ static void _omap_dispc_initial_config(void)
>>  	dispc_ovl_enable_zorder_planes();
>>  }
>>  
>> +static struct dispc_reg_fields omap2_dispc_reg_fields = {
>> +	.firhinc		=	{ 11,  0 },
>> +	.firvinc		=	{ 27, 16 },
>> +	.fifo_low_thresh	=	{  8,  0 },
>> +	.fifo_high_thresh	=	{ 24, 16 },
>> +	.fifosize		=	{  8,  0 },
>> +	.hori_accu		=	{  9,  0 },
>> +	.vert_accu		=	{ 25, 16 },
>> +};
> 
> And these tables can be consts.
> 
>> +static struct dispc_reg_fields omap3_dispc_reg_fields = {
>> +	.firhinc		=	{ 12,  0 },
>> +	.firvinc		=	{ 28, 16 },
>> +	.fifo_low_thresh	=	{ 11,  0 },
>> +	.fifo_high_thresh	=	{ 27, 16 },
>> +	.fifosize		=	{ 10,  0 },
>> +	.hori_accu		=	{  9,  0 },
>> +	.vert_accu		=	{ 25, 16 },
>> +};
>> +
>> +static struct dispc_reg_fields omap4_dispc_reg_fields = {
>> +	.firhinc		=	{ 12,  0 },
>> +	.firvinc		=	{ 28, 16 },
>> +	.fifo_low_thresh	=	{ 15,  0 },
>> +	.fifo_high_thresh	=	{ 31, 16 },
>> +	.fifosize		=	{ 15,  0 },
>> +	.hori_accu		=	{ 10,  0 },
>> +	.vert_accu		=	{ 26, 16 },
>> +};
>> +
>> +static struct dispc_reg_fields omap5_dispc_reg_fields = {
>> +	.firhinc		=	{ 12,  0 },
>> +	.firvinc		=	{ 28, 16 },
>> +	.fifo_low_thresh	=	{ 15,  0 },
>> +	.fifo_high_thresh	=	{ 31, 16 },
>> +	.fifosize		=	{ 15,  0 },
>> +	.hori_accu		=	{ 10,  0 },
>> +	.vert_accu		=	{ 26, 16 },
>> +};
>> +
>>  static const struct dispc_features omap24xx_dispc_feats __initconst = {
>>  	.sw_start		=	5,
>>  	.fp_start		=	15,
>> @@ -4065,6 +4116,7 @@ static const struct dispc_features omap24xx_dispc_feats __initconst = {
>>  	.no_framedone_tv	=	true,
>>  	.buffer_size_unit	=	1,
>>  	.burst_size_unit	=	8,
>> +	.reg_fields		=	&omap2_dispc_reg_fields,
>>  };
>>  
>>  static const struct dispc_features omap34xx_rev1_0_dispc_feats __initconst = {
>> @@ -4084,6 +4136,7 @@ static const struct dispc_features omap34xx_rev1_0_dispc_feats __initconst = {
>>  	.no_framedone_tv	=	true,
>>  	.buffer_size_unit	=	1,
>>  	.burst_size_unit	=	8,
>> +	.reg_fields		=	&omap3_dispc_reg_fields,
>>  };
>>  
>>  static const struct dispc_features omap34xx_rev3_0_dispc_feats __initconst = {
>> @@ -4103,6 +4156,7 @@ static const struct dispc_features omap34xx_rev3_0_dispc_feats __initconst = {
>>  	.no_framedone_tv	=	true,
>>  	.buffer_size_unit	=	1,
>>  	.burst_size_unit	=	8,
>> +	.reg_fields		=	&omap3_dispc_reg_fields,
>>  };
>>  
>>  static const struct dispc_features omap44xx_dispc_feats __initconst = {
>> @@ -4122,6 +4176,7 @@ static const struct dispc_features omap44xx_dispc_feats __initconst = {
>>  	.gfx_fifo_workaround	=	true,
>>  	.buffer_size_unit	=	16,
>>  	.burst_size_unit	=	16,
>> +	.reg_fields		=	&omap4_dispc_reg_fields,
>>  };
>>  
>>  static const struct dispc_features omap54xx_dispc_feats __initconst = {
>> @@ -4141,6 +4196,7 @@ static const struct dispc_features omap54xx_dispc_feats __initconst = {
>>  	.gfx_fifo_workaround	=	true,
>>  	.buffer_size_unit	=	16,
>>  	.burst_size_unit	=	16,
>> +	.reg_fields		=	&omap5_dispc_reg_fields,
>>  };
> 
> There's one thing to note here (and the same applies to DSI patches).
> The *_dispc_feats tables above are __initconst, and we make a copy of
> the needed table at probe time, so that the unneeded tables can be
> discarded. Now you add new tables, but they are not handled the same
> way. This is not a bug, but it's a bit inconsistent.
> 
> So I think we have three options:
> 
> - Make the new tables also __initconst, and create a copy of the needed
> tables, and fix up the pointers to point to the copied tables.
> 
> - Embed the new tables into the *_dispc_feats table, as you suggested
> previously. This would mean multiple copies of the same data in some cases.
> 
> - Remove the __initconst and the copy code.
> 
> I'm not sure which one to pick. The first one feels a bit complex, but
> perhaps it should be tried first to see how the actual code would look
> like. If it's just a few lines per table, I guess it's ok.
> 
>  Tomi
> 
> 

I would like to go with first approach. The code will look like this

static int __init dispc_init_features(struct platform_device *pdev)
{
.....................................
	const struct dispc_param_ranges *src_param;
	struct dispc_param_ranges *dst_param;

	dst_param = devm_kzalloc(&pdev->dev, sizeof(*dst_param), 	GFP_KERNEL);
	if (!dst_param) {
		dev_err(&pdev->dev, "Failed to allocate DISPC Param Ranges\n");
		return -ENOMEM;
	}

	switch (omapdss_get_version()) {
 	case OMAPDSS_VER_OMAP24xx:
 		src = &omap24xx_dispc_feats;
		src_param = &omap2_dispc_param_ranges;
 		break;
.............................
	}

	memcpy(dst, src, sizeof(*dst));
	memcpy(dst_param, src_param, sizeof(*dst_param));
	dst->params = dst_param;

 	dispc.feat = dst;

	return 0;
}

Yes, the code looks a bit complex but still well manageable.

-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

^ permalink raw reply

* Re: [PATCH V2 4/6] OMAPDSS: DSI: Move DSI specific reg_fields to dsi_feats
From: Chandrabhanu Mahapatra @ 2012-12-19  9:10 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <50CF0EA6.2040302@ti.com>

On Monday 17 December 2012 05:53 PM, Tomi Valkeinen wrote:
> Hi,
> 
> On 2012-12-05 12:16, Chandrabhanu Mahapatra wrote:
>> The DSI specific dss_reg_fields are moved to corresponding dsi_reg_fields
>> initialized in dsi_feats. The dsi_feats structure is initialized as per
>> corresponding DSS version in dsi_init_features().
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  drivers/video/omap2/dss/dsi.c          |  126 +++++++++++++++++++++++++++++---
>>  drivers/video/omap2/dss/dss_features.c |   16 ----
>>  drivers/video/omap2/dss/dss_features.h |    4 -
>>  3 files changed, 114 insertions(+), 32 deletions(-)
>>
> 
>> +static int __init dsi_init_features(struct platform_device *dsidev)
>> +{
>> +	const struct feats *src;
>> +	struct feats *dst;
>> +
>> +	dst = devm_kzalloc(&dsidev->dev, sizeof(*dst), GFP_KERNEL);
>> +	if (!dst) {
>> +		dev_err(&dsidev->dev, "Failed to allocate DISPC Features\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	switch (omapdss_get_version()) {
>> +	case OMAPDSS_VER_OMAP24xx:
>> +		src = &omap24xx_dsi_feats;
>> +		break;
>> +
>> +	case OMAPDSS_VER_OMAP34xx_ES1:
>> +	case OMAPDSS_VER_OMAP34xx_ES3:
>> +	case OMAPDSS_VER_OMAP3630:
>> +	case OMAPDSS_VER_AM35xx:
>> +		src = &omap34xx_dsi_feats;
>> +		break;
>> +
>> +	case OMAPDSS_VER_OMAP4430_ES1:
>> +	case OMAPDSS_VER_OMAP4430_ES2:
>> +	case OMAPDSS_VER_OMAP4:
>> +		src = &omap44xx_dsi_feats;
>> +		break;
>> +
>> +	case OMAPDSS_VER_OMAP5:
>> +		src = &omap54xx_dsi_feats;
>> +		break;
>> +
>> +	default:
>> +		return -ENODEV;
>> +	}
> 
> There's no DSI on OMAP2, so that case can be left out.
> 
>  Tomi
> 
> 

Ok. Forgot to see that in dss_features.

-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

^ permalink raw reply

* Fw: BUSINESS PROPOSAL, PLEASE VIEW ATTACHMENT
From: Engr @ 2012-12-18 20:09 UTC (permalink / raw)
  To: linux-fbdev

[-- Attachment #1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2: BUSINESS_PROPOSAL_FROM_ENGINEER ANTHONY.doc --]
[-- Type: application/msword, Size: 40448 bytes --]

^ permalink raw reply

* [PATCHv16 7/7] drm_modes: add of_videomode helpers
From: Steffen Trumtrar @ 2012-12-18 17:04 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355850256-16135-1-git-send-email-s.trumtrar@pengutronix.de>

Add helper to get drm_display_mode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/drm_modes.c |   33 +++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    4 ++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 184a22d..fd53454 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <video/of_videomode.h>
 #include <video/videomode.h>
 
 /**
@@ -541,6 +542,38 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
 EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+/**
+ * of_get_drm_display_mode - get a drm_display_mode from devicetree
+ * @np: device_node with the timing specification
+ * @dmode: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings and
+ * work with that instead.
+ */
+int of_get_drm_display_mode(struct device_node *np,
+			    struct drm_display_mode *dmode, int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	drm_display_mode_from_videomode(&vm, dmode);
+
+	pr_debug("%s: got %dx%d display mode from %s\n",
+		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+	drm_mode_debug_printmodeline(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 5fbb0fe..e26ca59 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -85,6 +85,7 @@ struct module;
 struct drm_file;
 struct drm_device;
 
+struct device_node;
 struct videomode;
 
 #include <drm/drm_os_linux.h>
@@ -1458,6 +1459,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 
 extern int drm_display_mode_from_videomode(const struct videomode *vm,
 					   struct drm_display_mode *dmode);
+extern int of_get_drm_display_mode(struct device_node *np,
+				   struct drm_display_mode *dmode,
+				   int index);
 
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 6/7] drm_modes: add videomode helpers
From: Steffen Trumtrar @ 2012-12-18 17:04 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355850256-16135-1-git-send-email-s.trumtrar@pengutronix.de>

Add conversion from videomode to drm_display_mode

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/drm_modes.c |   37 +++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    5 +++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..184a22d 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <video/videomode.h>
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -504,6 +505,42 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
 }
 EXPORT_SYMBOL(drm_gtf_mode);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int drm_display_mode_from_videomode(const struct videomode *vm,
+				    struct drm_display_mode *dmode)
+{
+	dmode->hdisplay = vm->hactive;
+	dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
+	dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
+	dmode->htotal = dmode->hsync_end + vm->hback_porch;
+
+	dmode->vdisplay = vm->vactive;
+	dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
+	dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
+	dmode->vtotal = dmode->vsync_end + vm->vback_porch;
+
+	dmode->clock = vm->pixelclock / 1000;
+
+	dmode->flags = 0;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_PHSYNC;
+	else if (vm->dmt_flags & VESA_DMT_HSYNC_LOW)
+		dmode->flags |= DRM_MODE_FLAG_NHSYNC;
+	if (vm->dmt_flags & VESA_DMT_VSYNC_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_PVSYNC;
+	else if (vm->dmt_flags & VESA_DMT_VSYNC_LOW)
+		dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+		dmode->flags |= DRM_MODE_FLAG_INTERLACE;
+	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
+	drm_mode_set_name(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3fd8280..5fbb0fe 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -85,6 +85,8 @@ struct module;
 struct drm_file;
 struct drm_device;
 
+struct videomode;
+
 #include <drm/drm_os_linux.h>
 #include <drm/drm_hashtab.h>
 #include <drm/drm_mm.h>
@@ -1454,6 +1456,9 @@ extern struct drm_display_mode *
 drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 				  struct drm_cmdline_mode *cmd);
 
+extern int drm_display_mode_from_videomode(const struct videomode *vm,
+					   struct drm_display_mode *dmode);
+
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
 extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 5/7] fbmon: add of_videomode helpers
From: Steffen Trumtrar @ 2012-12-18 17:04 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355850256-16135-1-git-send-email-s.trumtrar@pengutronix.de>

Add helper to get fb_videomode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/fbmon.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    4 ++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 17ce135..94ad0f7 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <video/of_videomode.h>
 #include <video/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
@@ -1425,6 +1426,47 @@ int fb_videomode_from_videomode(const struct videomode *vm,
 EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(const struct fb_videomode *m)
+{
+	pr_debug("fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u %u\n",
+		 m->xres, m->yres, m->refresh, m->pixclock, m->left_margin,
+		 m->right_margin, m->upper_margin, m->lower_margin,
+		 m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+			int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	fb_videomode_from_videomode(&vm, fb);
+
+	pr_debug("%s: got %dx%d display mode from %s\n",
+		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+	dump_fb_videomode(fb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 100a176..58b9860 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -20,6 +20,7 @@ struct fb_info;
 struct device;
 struct file;
 struct videomode;
+struct device_node;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF	1
@@ -715,6 +716,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int of_get_fb_videomode(struct device_node *np,
+			       struct fb_videomode *fb,
+			       int index);
 extern int fb_videomode_from_videomode(const struct videomode *vm,
 				       struct fb_videomode *fbmode);
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 4/7] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2012-12-18 17:04 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355850256-16135-1-git-send-email-s.trumtrar@pengutronix.de>

Add a function to convert from the generic videomode to a fb_videomode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    4 ++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..17ce135 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <video/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
@@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
 	kfree(timings);
 	return err;
 }
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(const struct videomode *vm,
+				struct fb_videomode *fbmode)
+{
+	unsigned int htotal, vtotal;
+
+	fbmode->xres = vm->hactive;
+	fbmode->left_margin = vm->hback_porch;
+	fbmode->right_margin = vm->hfront_porch;
+	fbmode->hsync_len = vm->hsync_len;
+
+	fbmode->yres = vm->vactive;
+	fbmode->upper_margin = vm->vback_porch;
+	fbmode->lower_margin = vm->vfront_porch;
+	fbmode->vsync_len = vm->vsync_len;
+
+	/* prevent division by zero in KHZ2PICOS macro */
+	fbmode->pixclock = vm->pixelclock ?
+			KHZ2PICOS(vm->pixelclock / 1000) : 0;
+
+	fbmode->sync = 0;
+	fbmode->vmode = 0;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+		fbmode->vmode |= FB_VMODE_INTERLACED;
+	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+		fbmode->vmode |= FB_VMODE_DOUBLE;
+	fbmode->flag = 0;
+
+	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+		 vm->hsync_len;
+	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+		 vm->vsync_len;
+	/* prevent division by zero */
+	if (htotal && vtotal) {
+		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
+	/* a mode must have htotal and vtotal != 0 or it is invalid */
+	} else {
+		fbmode->refresh = 0;
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c7a9571..100a176 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -19,6 +19,7 @@ struct vm_area_struct;
 struct fb_info;
 struct device;
 struct file;
+struct videomode;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF	1
@@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int fb_videomode_from_videomode(const struct videomode *vm,
+				       struct fb_videomode *fbmode);
+
 /* drivers/video/modedb.c */
 #define VESA_MODEDB_SIZE 34
 extern void fb_var_to_videomode(struct fb_videomode *mode,
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 3/7] video: add of helper for display timings/videomode
From: Steffen Trumtrar @ 2012-12-18 17:04 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Philipp Zabel, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, kernel, Florian Tobias Schandinat, David Airlie,
	Rob Clark, Leela Krishna Amudala
In-Reply-To: <1355850256-16135-1-git-send-email-s.trumtrar@pengutronix.de>

This adds support for reading display timings from DT into a struct
display_timings. The of_display_timing implementation supports multiple
subnodes. All children are read into an array, that can be queried.

If no native mode is specified, the first subnode will be used.

For cases where the graphics driver knows there can be only one
mode description or where the driver only supports one mode, a helper
function of_get_videomode is added, that gets a struct videomode from DT.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
 drivers/video/Kconfig                              |   15 ++
 drivers/video/Makefile                             |    2 +
 drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   54 +++++
 include/video/of_display_timing.h                  |   20 ++
 include/video/of_videomode.h                       |   18 ++
 7 files changed, 457 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 include/video/of_display_timing.h
 create mode 100644 include/video/of_videomode.h

diff --git a/Documentation/devicetree/bindings/video/display-timing.txt b/Documentation/devicetree/bindings/video/display-timing.txt
new file mode 100644
index 0000000..1500385
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timing.txt
@@ -0,0 +1,109 @@
+display-timing bindings
+===========+
+display-timings node
+--------------------
+
+required properties:
+ - none
+
+optional properties:
+ - native-mode: The native mode for the display, in case multiple modes are
+		provided. When omitted, assume the first node is the native.
+
+timing subnode
+--------------
+
+required properties:
+ - hactive, vactive: display resolution
+ - hfront-porch, hback-porch, hsync-len: horizontal display timing parameters
+   in pixels
+   vfront-porch, vback-porch, vsync-len: vertical display timing parameters in
+   lines
+ - clock-frequency: display clock in Hz
+
+optional properties:
+ - hsync-active: hsync pulse is active low/high/ignored
+ - vsync-active: vsync pulse is active low/high/ignored
+ - de-active: data-enable pulse is active low/high/ignored
+ - pixelclk-active: with
+			- active high = drive pixel data on rising edge/
+					sample data on falling edge
+			- active low  = drive pixel data on falling edge/
+					sample data on rising edge
+			- ignored     = ignored
+ - interlaced (bool): boolean to enable interlaced mode
+ - doublescan (bool): boolean to enable doublescan mode
+
+All the optional properties that are not bool follow the following logic:
+    <1>: high active
+    <0>: low active
+    omitted: not used on hardware
+
+There are different ways of describing the capabilities of a display. The
+devicetree representation corresponds to the one commonly found in datasheets
+for displays. If a display supports multiple signal timings, the native-mode
+can be specified.
+
+The parameters are defined as:
+
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vback_porch                 |          |       |
+  |          |        ↓                            |          |       |
+  +----------#######################################----------+-------+
+  |          #        ↑                            #          |       |
+  |          #        |                            #          |       |
+  |  hback   #        |                            #  hfront  | hsync |
+  |   porch  #        |       hactive              #  porch   |  len  |
+  |<-------->#<-------+--------------------------->#<-------->|<----->|
+  |          #        |                            #          |       |
+  |          #        |vactive                     #          |       |
+  |          #        |                            #          |       |
+  |          #        ↓                            #          |       |
+  +----------#######################################----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vfront_porch                |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vsync_len                   |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+
+Example:
+
+	display-timings {
+		native-mode = <&timing0>;
+		timing0: 1080p24 {
+			/* 1920x1080p24 */
+			clock-frequency = <52000000>;
+			hactive = <1920>;
+			vactive = <1080>;
+			hfront-porch = <25>;
+			hback-porch = <25>;
+			hsync-len = <25>;
+			vback-porch = <2>;
+			vfront-porch = <2>;
+			vsync-len = <2>;
+			hsync-active = <1>;
+		};
+	};
+
+Every required property also supports the use of ranges, so the commonly used
+datasheet description with minimum, typical and maximum values can be used.
+
+Example:
+
+	timing1: timing {
+		/* 1920x1080p24 */
+		clock-frequency = <148500000>;
+		hactive = <1920>;
+		vactive = <1080>;
+		hsync-len = <0 44 60>;
+		hfront-porch = <80 88 95>;
+		hback-porch = <100 148 160>;
+		vfront-porch = <0 4 6>;
+		vback-porch = <0 36 50>;
+		vsync-len = <0 5 6>;
+	};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2a23b18..c000f5a 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -39,6 +39,21 @@ config DISPLAY_TIMING
 config VIDEOMODE
        bool
 
+config OF_DISPLAY_TIMING
+	bool "Enable device tree display timing support"
+	depends on OF
+	select DISPLAY_TIMING
+	help
+	  helper to parse display timings from the devicetree
+
+config OF_VIDEOMODE
+	bool "Enable device tree videomode support"
+	depends on OF
+	select VIDEOMODE
+	select OF_DISPLAY_TIMING
+	help
+	  helper to get videomodes from the devicetree
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index fc30439..b936b00 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -168,4 +168,6 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
 obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o
 obj-$(CONFIG_VIDEOMODE) += videomode.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
new file mode 100644
index 0000000..13ecd98
--- /dev/null
+++ b/drivers/video/of_display_timing.c
@@ -0,0 +1,239 @@
+/*
+ * OF helpers for parsing display timings
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/export.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+
+/**
+ * parse_timing_property - parse timing_entry from device_node
+ * @np: device_node with the property
+ * @name: name of the property
+ * @result: will be set to the return value
+ *
+ * DESCRIPTION:
+ * Every display_timing can be specified with either just the typical value or
+ * a range consisting of min/typ/max. This function helps handling this
+ **/
+static int parse_timing_property(struct device_node *np, const char *name,
+			  struct timing_entry *result)
+{
+	struct property *prop;
+	int length, cells, ret;
+
+	prop = of_find_property(np, name, &length);
+	if (!prop) {
+		pr_err("%s: could not find property %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	cells = length / sizeof(u32);
+	if (cells = 1) {
+		ret = of_property_read_u32(np, name, &result->typ);
+		result->min = result->typ;
+		result->max = result->typ;
+	} else if (cells = 3) {
+		ret = of_property_read_u32_array(np, name, &result->min, cells);
+	} else {
+		pr_err("%s: illegal timing specification in %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+/**
+ * of_get_display_timing - parse display_timing entry from device_node
+ * @np: device_node with the properties
+ **/
+static struct display_timing *of_get_display_timing(struct device_node *np)
+{
+	struct display_timing *dt;
+	u32 val = 0;
+	int ret = 0;
+
+	dt = kzalloc(sizeof(*dt), GFP_KERNEL);
+	if (!dt) {
+		pr_err("%s: could not allocate display_timing struct\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	ret |= parse_timing_property(np, "hback-porch", &dt->hback_porch);
+	ret |= parse_timing_property(np, "hfront-porch", &dt->hfront_porch);
+	ret |= parse_timing_property(np, "hactive", &dt->hactive);
+	ret |= parse_timing_property(np, "hsync-len", &dt->hsync_len);
+	ret |= parse_timing_property(np, "vback-porch", &dt->vback_porch);
+	ret |= parse_timing_property(np, "vfront-porch", &dt->vfront_porch);
+	ret |= parse_timing_property(np, "vactive", &dt->vactive);
+	ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
+	ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
+
+	dt->dmt_flags = 0;
+	dt->data_flags = 0;
+	if (!of_property_read_u32(np, "vsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_VSYNC_HIGH :
+				VESA_DMT_VSYNC_LOW;
+	if (!of_property_read_u32(np, "hsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_HSYNC_HIGH :
+				VESA_DMT_HSYNC_LOW;
+	if (!of_property_read_u32(np, "de-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_DE_HIGH :
+				DISPLAY_FLAGS_DE_LOW;
+	if (!of_property_read_u32(np, "pixelclk-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_PIXDATA_POSEDGE :
+				DISPLAY_FLAGS_PIXDATA_NEGEDGE;
+
+	if (of_property_read_bool(np, "interlaced"))
+		dt->data_flags |= DISPLAY_FLAGS_INTERLACED;
+	if (of_property_read_bool(np, "doublescan"))
+		dt->data_flags |= DISPLAY_FLAGS_DOUBLESCAN;
+
+	if (ret) {
+		pr_err("%s: error reading timing properties\n",
+			of_node_full_name(np));
+		kfree(dt);
+		return NULL;
+	}
+
+	return dt;
+}
+
+/**
+ * of_get_display_timings - parse all display_timing entries from a device_node
+ * @np: device_node with the subnodes
+ **/
+struct display_timings *of_get_display_timings(struct device_node *np)
+{
+	struct device_node *timings_np;
+	struct device_node *entry;
+	struct device_node *native_mode;
+	struct display_timings *disp;
+
+	if (!np) {
+		pr_err("%s: no devicenode given\n", of_node_full_name(np));
+		return NULL;
+	}
+
+	timings_np = of_find_node_by_name(np, "display-timings");
+	if (!timings_np) {
+		pr_err("%s: could not find display-timings node\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	disp = kzalloc(sizeof(*disp), GFP_KERNEL);
+	if (!disp) {
+		pr_err("%s: could not allocate struct disp'\n",
+			of_node_full_name(np));
+		goto dispfail;
+	}
+
+	entry = of_parse_phandle(timings_np, "native-mode", 0);
+	/* assume first child as native mode if none provided */
+	if (!entry)
+		entry = of_get_next_child(np, NULL);
+	/* if there is no child, it is useless to go on */
+	if (!entry) {
+		pr_err("%s: no timing specifications given\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	pr_debug("%s: using %s as default timing\n",
+		of_node_full_name(np), entry->name);
+
+	native_mode = entry;
+
+	disp->num_timings = of_get_child_count(timings_np);
+	if (disp->num_timings = 0) {
+		/* should never happen, as entry was already found above */
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->timings = kzalloc(sizeof(struct display_timing *) *
+				disp->num_timings, GFP_KERNEL);
+	if (!disp->timings) {
+		pr_err("%s: could not allocate timings array\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->num_timings = 0;
+	disp->native_mode = 0;
+
+	for_each_child_of_node(timings_np, entry) {
+		struct display_timing *dt;
+
+		dt = of_get_display_timing(entry);
+		if (!dt) {
+			/*
+			 * to not encourage wrong devicetrees, fail in case of
+			 * an error
+			 */
+			pr_err("%s: error in timing %d\n",
+				of_node_full_name(np), disp->num_timings + 1);
+			goto timingfail;
+		}
+
+		if (native_mode = entry)
+			disp->native_mode = disp->num_timings;
+
+		disp->timings[disp->num_timings] = dt;
+		disp->num_timings++;
+	}
+	of_node_put(timings_np);
+	/*
+	 * native_mode points to the device_node returned by of_parse_phandle
+	 * therefore call of_node_put on it
+	 */
+	of_node_put(native_mode);
+
+	pr_debug("%s: got %d timings. Using timing #%d as default\n",
+		of_node_full_name(np), disp->num_timings,
+		disp->native_mode + 1);
+
+	return disp;
+
+timingfail:
+	if (native_mode)
+		of_node_put(native_mode);
+	display_timings_release(disp);
+entryfail:
+	kfree(disp);
+dispfail:
+	of_node_put(timings_np);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timings);
+
+/**
+ * of_display_timings_exist - check if a display-timings node is provided
+ * @np: device_node with the timing
+ **/
+int of_display_timings_exist(struct device_node *np)
+{
+	struct device_node *timings_np;
+
+	if (!np)
+		return -EINVAL;
+
+	timings_np = of_parse_phandle(np, "display-timings", 0);
+	if (!timings_np)
+		return -EINVAL;
+
+	of_node_put(timings_np);
+	return 1;
+}
+EXPORT_SYMBOL_GPL(of_display_timings_exist);
diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c
new file mode 100644
index 0000000..5b8066c
--- /dev/null
+++ b/drivers/video/of_videomode.c
@@ -0,0 +1,54 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/of.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+/**
+ * of_get_videomode - get the videomode #<index> from devicetree
+ * @np - devicenode with the display_timings
+ * @vm - set to return value
+ * @index - index into list of display_timings
+ *	    (Set this to OF_USE_NATIVE_MODE to use whatever mode is
+ *	     specified as native mode in the DT.)
+ *
+ * DESCRIPTION:
+ * Get a list of all display timings and put the one
+ * specified by index into *vm. This function should only be used, if
+ * only one videomode is to be retrieved. A driver that needs to work
+ * with multiple/all videomodes should work with
+ * of_get_display_timings instead.
+ **/
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index)
+{
+	struct display_timings *disp;
+	int ret;
+
+	disp = of_get_display_timings(np);
+	if (!disp) {
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		return -EINVAL;
+	}
+
+	if (index = OF_USE_NATIVE_MODE)
+		index = disp->native_mode;
+
+	ret = videomode_from_timing(disp, vm, index);
+	if (ret)
+		return ret;
+
+	display_timings_release(disp);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
diff --git a/include/video/of_display_timing.h b/include/video/of_display_timing.h
new file mode 100644
index 0000000..8016eb7
--- /dev/null
+++ b/include/video/of_display_timing.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * display timings of helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMING_H
+#define __LINUX_OF_DISPLAY_TIMING_H
+
+struct device_node;
+struct display_timings;
+
+#define OF_USE_NATIVE_MODE -1
+
+struct display_timings *of_get_display_timings(struct device_node *np);
+int of_display_timings_exist(struct device_node *np);
+
+#endif
diff --git a/include/video/of_videomode.h b/include/video/of_videomode.h
new file mode 100644
index 0000000..a07efcc
--- /dev/null
+++ b/include/video/of_videomode.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * videomode of-helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+struct device_node;
+struct videomode;
+
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index);
+
+#endif /* __LINUX_OF_VIDEOMODE_H */
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 2/7] video: add display_timing and videomode
From: Steffen Trumtrar @ 2012-12-18 17:04 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355850256-16135-1-git-send-email-s.trumtrar@pengutronix.de>

Add display_timing structure and the according helper functions. This allows
the description of a display via its supported timing parameters.

Also, add helper functions to convert from display timings to a generic videomode
structure.

The struct display_timing specifies all needed parameters to describe the signal
properties of a display in one mode. This includes
    - ranges for signals that may have min-, max- and typical values
    - single integers for signals that can be on, off or are ignored
    - booleans for signals that are either on or off

As a display may support multiple modes like this, a struct display_timings is
added, that holds all given struct display_timing pointers and declares the
native mode of the display.

Although a display may state that a signal can be in a range, it is driven with
fixed values that indicate a videomode. Therefore graphic drivers don't need all
the information of struct display_timing, but would generate a videomode from
the given set of supported signal timings and work with that.

The video subsystems all define their own structs that describe a mode and work
with that (e.g. fb_videomode or drm_display_mode). To slowly replace all those
various structures and allow code reuse across those subsystems, add struct
videomode as a generic description.

This patch only includes the most basic fields in struct videomode. All missing
fields that are needed to have a really generic video mode description can be
added at a later stage.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/Kconfig          |    6 ++
 drivers/video/Makefile         |    2 +
 drivers/video/display_timing.c |   24 ++++++++
 drivers/video/videomode.c      |   39 +++++++++++++
 include/video/display_timing.h |  124 ++++++++++++++++++++++++++++++++++++++++
 include/video/videomode.h      |   48 ++++++++++++++++
 6 files changed, 243 insertions(+)
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/video/display_timing.h
 create mode 100644 include/video/videomode.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d08d799..2a23b18 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL
 	  This framework adds support for low-level control of the video 
 	  output switch.
 
+config DISPLAY_TIMING
+       bool
+
+config VIDEOMODE
+       bool
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 23e948e..fc30439 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -167,3 +167,5 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 0000000..5e1822c
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+
+void display_timings_release(struct display_timings *disp)
+{
+	if (disp->timings) {
+		unsigned int i;
+
+		for (i = 0; i < disp->num_timings; i++)
+			kfree(disp->timings[i]);
+		kfree(disp->timings);
+	}
+	kfree(disp);
+}
+EXPORT_SYMBOL_GPL(display_timings_release);
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 0000000..21c47a2
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,39 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <video/display_timing.h>
+#include <video/videomode.h>
+
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index)
+{
+	struct display_timing *dt;
+
+	dt = display_timings_get(disp, index);
+	if (!dt)
+		return -EINVAL;
+
+	vm->pixelclock = display_timing_get_value(&dt->pixelclock, TE_TYP);
+	vm->hactive = display_timing_get_value(&dt->hactive, TE_TYP);
+	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, TE_TYP);
+	vm->hback_porch = display_timing_get_value(&dt->hback_porch, TE_TYP);
+	vm->hsync_len = display_timing_get_value(&dt->hsync_len, TE_TYP);
+
+	vm->vactive = display_timing_get_value(&dt->vactive, TE_TYP);
+	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, TE_TYP);
+	vm->vback_porch = display_timing_get_value(&dt->vback_porch, TE_TYP);
+	vm->vsync_len = display_timing_get_value(&dt->vsync_len, TE_TYP);
+
+	vm->dmt_flags = dt->dmt_flags;
+	vm->data_flags = dt->data_flags;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/video/display_timing.h b/include/video/display_timing.h
new file mode 100644
index 0000000..71e9a38
--- /dev/null
+++ b/include/video/display_timing.h
@@ -0,0 +1,124 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_DISPLAY_TIMING_H
+#define __LINUX_DISPLAY_TIMING_H
+
+#include <linux/bitops.h>
+#include <linux/types.h>
+
+/* VESA display monitor timing parameters */
+#define VESA_DMT_HSYNC_LOW		BIT(0)
+#define VESA_DMT_HSYNC_HIGH		BIT(1)
+#define VESA_DMT_VSYNC_LOW		BIT(2)
+#define VESA_DMT_VSYNC_HIGH		BIT(3)
+
+/* display specific flags */
+#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
+#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
+#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
+#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
+#define DISPLAY_FLAGS_INTERLACED	BIT(4)
+#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
+
+/*
+ * A single signal can be specified via a range of minimal and maximal values
+ * with a typical value, that lies somewhere inbetween.
+ */
+struct timing_entry {
+	u32 min;
+	u32 typ;
+	u32 max;
+};
+
+enum timing_entry_index {
+	TE_MIN = 0,
+	TE_TYP = 1,
+	TE_MAX = 2,
+};
+
+/*
+ * Single "mode" entry. This describes one set of signal timings a display can
+ * have in one setting. This struct can later be converted to struct videomode
+ * (see include/video/videomode.h). As each timing_entry can be defined as a
+ * range, one struct display_timing may become multiple struct videomodes.
+ *
+ * Example: hsync active high, vsync active low
+ *
+ *				    Active Video
+ * Video  ______________________XXXXXXXXXXXXXXXXXXXXXX_____________________
+ *	  |<- sync ->|<- back ->|<----- active ----->|<- front ->|<- sync..
+ *	  |	     |	 porch  |		     |	 porch	 |
+ *
+ * HSync _|¯¯¯¯¯¯¯¯¯¯|___________________________________________|¯¯¯¯¯¯¯¯¯
+ *
+ * VSync ¯|__________|¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯|_________
+ */
+struct display_timing {
+	struct timing_entry pixelclock;
+
+	struct timing_entry hactive;		/* hor. active video */
+	struct timing_entry hfront_porch;	/* hor. front porch */
+	struct timing_entry hback_porch;	/* hor. back porch */
+	struct timing_entry hsync_len;		/* hor. sync len */
+
+	struct timing_entry vactive;		/* ver. active video */
+	struct timing_entry vfront_porch;	/* ver. front porch */
+	struct timing_entry vback_porch;	/* ver. back porch */
+	struct timing_entry vsync_len;		/* ver. sync len */
+
+	unsigned int dmt_flags;			/* VESA DMT flags */
+	unsigned int data_flags;		/* video data flags */
+};
+
+/*
+ * This describes all timing settings a display provides.
+ * The native_mode is the default setting for this display.
+ * Drivers that can handle multiple videomodes should work with this struct and
+ * convert each entry to the desired end result.
+ */
+struct display_timings {
+	unsigned int num_timings;
+	unsigned int native_mode;
+
+	struct display_timing **timings;
+};
+
+/* get value specified by index from struct timing_entry */
+static inline u32 display_timing_get_value(const struct timing_entry *te,
+					   enum timing_entry_index index)
+{
+	switch (index) {
+	case TE_MIN:
+		return te->min;
+		break;
+	case TE_TYP:
+		return te->typ;
+		break;
+	case TE_MAX:
+		return te->max;
+		break;
+	default:
+		return te->typ;
+	}
+}
+
+/* get one entry from struct display_timings */
+static inline struct display_timing *display_timings_get(const struct
+							 display_timings *disp,
+							 unsigned int index)
+{
+	if (disp->num_timings > index)
+		return disp->timings[index];
+	else
+		return NULL;
+}
+
+void display_timings_release(struct display_timings *disp);
+
+#endif
diff --git a/include/video/videomode.h b/include/video/videomode.h
new file mode 100644
index 0000000..a421562
--- /dev/null
+++ b/include/video/videomode.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * generic videomode description
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_VIDEOMODE_H
+#define __LINUX_VIDEOMODE_H
+
+#include <linux/types.h>
+#include <video/display_timing.h>
+
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+	unsigned long pixelclock;	/* pixelclock in Hz */
+
+	u32 hactive;
+	u32 hfront_porch;
+	u32 hback_porch;
+	u32 hsync_len;
+
+	u32 vactive;
+	u32 vfront_porch;
+	u32 vback_porch;
+	u32 vsync_len;
+
+	unsigned int dmt_flags;	/* VESA DMT flags */
+	unsigned int data_flags; /* video data flags */
+};
+
+/**
+ * videomode_from_timing - convert display timing to videomode
+ * @disp: structure with all possible timing entries
+ * @vm: return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function converts a struct display_timing to a struct videomode.
+ */
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index);
+
+#endif
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 1/7] viafb: rename display_timing to via_display_timing
From: Steffen Trumtrar @ 2012-12-18 17:04 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355850256-16135-1-git-send-email-s.trumtrar@pengutronix.de>

The struct display_timing is specific to the via subsystem. The naming leads to
collisions with the new struct display_timing, which is supposed to be a shared
struct between different subsystems.
To clean this up, prepend the existing struct with the subsystem it is specific
to.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/via/hw.c              |    6 +++---
 drivers/video/via/hw.h              |    2 +-
 drivers/video/via/lcd.c             |    2 +-
 drivers/video/via/share.h           |    2 +-
 drivers/video/via/via_modesetting.c |    8 ++++----
 drivers/video/via/via_modesetting.h |    6 +++---
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 898590d..5563c67 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -1467,10 +1467,10 @@ void viafb_set_vclock(u32 clk, int set_iga)
 	via_write_misc_reg_mask(0x0C, 0x0C); /* select external clock */
 }
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres)
 {
-	struct display_timing timing;
+	struct via_display_timing timing;
 	u16 dx = (var->xres - cxres) / 2, dy = (var->yres - cyres) / 2;
 
 	timing.hor_addr = cxres;
@@ -1491,7 +1491,7 @@ struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga)
 {
-	struct display_timing crt_reg = var_to_timing(var,
+	struct via_display_timing crt_reg = var_to_timing(var,
 		cxres ? cxres : var->xres, cyres ? cyres : var->yres);
 
 	if (iga = IGA1)
diff --git a/drivers/video/via/hw.h b/drivers/video/via/hw.h
index 6be243c..c3f2572 100644
--- a/drivers/video/via/hw.h
+++ b/drivers/video/via/hw.h
@@ -637,7 +637,7 @@ extern int viafb_LCD_ON;
 extern int viafb_DVI_ON;
 extern int viafb_hotplug;
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres);
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga);
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 1650379..022b0df 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -549,7 +549,7 @@ void viafb_lcd_set_mode(const struct fb_var_screeninfo *var, u16 cxres,
 	int panel_hres = plvds_setting_info->lcd_panel_hres;
 	int panel_vres = plvds_setting_info->lcd_panel_vres;
 	u32 clock;
-	struct display_timing timing;
+	struct via_display_timing timing;
 	struct fb_var_screeninfo panel_var;
 	const struct fb_videomode *mode_crt_table, *panel_crt_table;
 
diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
index 3158dfc..65c65c6 100644
--- a/drivers/video/via/share.h
+++ b/drivers/video/via/share.h
@@ -319,7 +319,7 @@ struct crt_mode_table {
 	int refresh_rate;
 	int h_sync_polarity;
 	int v_sync_polarity;
-	struct display_timing crtc;
+	struct via_display_timing crtc;
 };
 
 struct io_reg {
diff --git a/drivers/video/via/via_modesetting.c b/drivers/video/via/via_modesetting.c
index 0e431ae..0b414b0 100644
--- a/drivers/video/via/via_modesetting.c
+++ b/drivers/video/via/via_modesetting.c
@@ -30,9 +30,9 @@
 #include "debug.h"
 
 
-void via_set_primary_timing(const struct display_timing *timing)
+void via_set_primary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total / 8 - 5;
 	raw.hor_addr = timing->hor_addr / 8 - 1;
@@ -88,9 +88,9 @@ void via_set_primary_timing(const struct display_timing *timing)
 	via_write_reg_mask(VIACR, 0x17, 0x80, 0x80);
 }
 
-void via_set_secondary_timing(const struct display_timing *timing)
+void via_set_secondary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total - 1;
 	raw.hor_addr = timing->hor_addr - 1;
diff --git a/drivers/video/via/via_modesetting.h b/drivers/video/via/via_modesetting.h
index 06e09fe..f6a6503 100644
--- a/drivers/video/via/via_modesetting.h
+++ b/drivers/video/via/via_modesetting.h
@@ -33,7 +33,7 @@
 #define VIA_PITCH_MAX	0x3FF8
 
 
-struct display_timing {
+struct via_display_timing {
 	u16 hor_total;
 	u16 hor_addr;
 	u16 hor_blank_start;
@@ -49,8 +49,8 @@ struct display_timing {
 };
 
 
-void via_set_primary_timing(const struct display_timing *timing);
-void via_set_secondary_timing(const struct display_timing *timing);
+void via_set_primary_timing(const struct via_display_timing *timing);
+void via_set_secondary_timing(const struct via_display_timing *timing);
 void via_set_primary_address(u32 addr);
 void via_set_secondary_address(u32 addr);
 void via_set_primary_pitch(u32 pitch);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 0/7] of: add display helper
From: Steffen Trumtrar @ 2012-12-18 17:04 UTC (permalink / raw)
  To: devicetree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355849873-8051-1-git-send-email-s.trumtrar@pengutronix.de>

Hi!

Finally, right in time before the end of the world on friday, v16 of the
display helpers.

Changes since v15:
        - move include/linux/{videomode,display_timing}.h to include/video
        - move include/linux/of_{videomode,display_timing}.h to include/video
        - reimplement flags: add VESA flags and data flags
        - let pixelclock in struct videomode be unsigned long
        - rename of_display_timings_exists to of_display_timings_exist
        - revise logging/error messages: replace __func__ with np->full_name
        - rename pixelclk-inverted to pixelclk-active
        - revise comments in code

Changes since v14:
        - fix "const struct *" warning
                (reported by: Leela Krishna Amudala <l.krishna@samsung.com>)
        - return -EINVAL when htotal or vtotal are zero
        - remove unreachable code in of_get_display_timings
        - include headers in .c files and not implicit in .h
        - sort includes alphabetically
        - fix lower/uppercase in binding documentation
        - rebase onto v3.7-rc7

Changes since v13:
        - fix "const struct *" warning
                (reported by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>)
        - prevent division by zero in fb_videomode_from_videomode

Changes since v12:
        - rename struct display_timing to via_display_timing in via subsystem
        - fix refreshrate calculation
        - fix "const struct *" warnings
                (reported by: Manjunathappa, Prakash <prakash.pm@ti.com>)
        - some CodingStyle fixes
        - rewrite parts of commit messages and display-timings.txt
        - let display_timing_get_value get all values instead of just typical

Changes since v11:
        - make pointers const where applicable
        - add reviewed-by Laurent Pinchart

Changes since v10:
        - fix function name (drm_)display_mode_from_videomode
        - add acked-by, reviewed-by, tested-by

Changes since v9:
        - don't leak memory when previous timings were correct
        - CodingStyle fixes
        - move blank lines around

Changes since v8:
        - fix memory leaks
        - change API to be more consistent (foo_from_bar(struct bar, struct foo))
        - include headers were necessary
        - misc minor bugfixes

Changes since v7:
        - move of_xxx to drivers/video
        - remove non-binding documentation from display-timings.txt
        - squash display_timings and videomode in one patch
        - misc minor fixes

Changes since v6:
        - get rid of some empty lines etc.
        - move functions to their subsystems
        - split of_ from non-of_ functions
        - add at least some kerneldoc to some functions

Changes since v5:
        - removed all display stuff and just describe timings

Changes since v4:
        - refactored functions

Changes since v3:
        - print error messages
        - free alloced memory
        - general cleanup

Changes since v2:
        - use hardware-near property-names
        - provide a videomode structure
        - allow ranges for all properties (<min,typ,max>)
        - functions to get display_mode or fb_videomode

Regards,
Steffen



Steffen Trumtrar (7):
  viafb: rename display_timing to via_display_timing
  video: add display_timing and videomode
  video: add of helper for display timings/videomode
  fbmon: add videomode helpers
  fbmon: add of_videomode helpers
  drm_modes: add videomode helpers
  drm_modes: add of_videomode helpers

 .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
 drivers/gpu/drm/drm_modes.c                        |   70 ++++++
 drivers/video/Kconfig                              |   21 ++
 drivers/video/Makefile                             |    4 +
 drivers/video/display_timing.c                     |   24 ++
 drivers/video/fbmon.c                              |   94 ++++++++
 drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   54 +++++
 drivers/video/via/hw.c                             |    6 +-
 drivers/video/via/hw.h                             |    2 +-
 drivers/video/via/lcd.c                            |    2 +-
 drivers/video/via/share.h                          |    2 +-
 drivers/video/via/via_modesetting.c                |    8 +-
 drivers/video/via/via_modesetting.h                |    6 +-
 drivers/video/videomode.c                          |   39 ++++
 include/drm/drmP.h                                 |    9 +
 include/linux/fb.h                                 |    8 +
 include/video/display_timing.h                     |  124 ++++++++++
 include/video/of_display_timing.h                  |   20 ++
 include/video/of_videomode.h                       |   18 ++
 include/video/videomode.h                          |   48 ++++
 21 files changed, 894 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/video/display_timing.h
 create mode 100644 include/video/of_display_timing.h
 create mode 100644 include/video/of_videomode.h
 create mode 100644 include/video/videomode.h

-- 
1.7.10.4


^ permalink raw reply

* [PATCHv16 7/7] drm_modes: add of_videomode helpers
From: Steffen Trumtrar @ 2012-12-18 16:57 UTC (permalink / raw)
  To: devicestree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355849873-8051-1-git-send-email-s.trumtrar@pengutronix.de>

Add helper to get drm_display_mode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/drm_modes.c |   33 +++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    4 ++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 184a22d..fd53454 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <video/of_videomode.h>
 #include <video/videomode.h>
 
 /**
@@ -541,6 +542,38 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
 EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+/**
+ * of_get_drm_display_mode - get a drm_display_mode from devicetree
+ * @np: device_node with the timing specification
+ * @dmode: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings and
+ * work with that instead.
+ */
+int of_get_drm_display_mode(struct device_node *np,
+			    struct drm_display_mode *dmode, int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	drm_display_mode_from_videomode(&vm, dmode);
+
+	pr_debug("%s: got %dx%d display mode from %s\n",
+		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+	drm_mode_debug_printmodeline(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 5fbb0fe..e26ca59 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -85,6 +85,7 @@ struct module;
 struct drm_file;
 struct drm_device;
 
+struct device_node;
 struct videomode;
 
 #include <drm/drm_os_linux.h>
@@ -1458,6 +1459,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 
 extern int drm_display_mode_from_videomode(const struct videomode *vm,
 					   struct drm_display_mode *dmode);
+extern int of_get_drm_display_mode(struct device_node *np,
+				   struct drm_display_mode *dmode,
+				   int index);
 
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 6/7] drm_modes: add videomode helpers
From: Steffen Trumtrar @ 2012-12-18 16:57 UTC (permalink / raw)
  To: devicestree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355849873-8051-1-git-send-email-s.trumtrar@pengutronix.de>

Add conversion from videomode to drm_display_mode

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/drm_modes.c |   37 +++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    5 +++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..184a22d 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <video/videomode.h>
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -504,6 +505,42 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
 }
 EXPORT_SYMBOL(drm_gtf_mode);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int drm_display_mode_from_videomode(const struct videomode *vm,
+				    struct drm_display_mode *dmode)
+{
+	dmode->hdisplay = vm->hactive;
+	dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
+	dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
+	dmode->htotal = dmode->hsync_end + vm->hback_porch;
+
+	dmode->vdisplay = vm->vactive;
+	dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
+	dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
+	dmode->vtotal = dmode->vsync_end + vm->vback_porch;
+
+	dmode->clock = vm->pixelclock / 1000;
+
+	dmode->flags = 0;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_PHSYNC;
+	else if (vm->dmt_flags & VESA_DMT_HSYNC_LOW)
+		dmode->flags |= DRM_MODE_FLAG_NHSYNC;
+	if (vm->dmt_flags & VESA_DMT_VSYNC_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_PVSYNC;
+	else if (vm->dmt_flags & VESA_DMT_VSYNC_LOW)
+		dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+		dmode->flags |= DRM_MODE_FLAG_INTERLACE;
+	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
+	drm_mode_set_name(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3fd8280..5fbb0fe 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -85,6 +85,8 @@ struct module;
 struct drm_file;
 struct drm_device;
 
+struct videomode;
+
 #include <drm/drm_os_linux.h>
 #include <drm/drm_hashtab.h>
 #include <drm/drm_mm.h>
@@ -1454,6 +1456,9 @@ extern struct drm_display_mode *
 drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 				  struct drm_cmdline_mode *cmd);
 
+extern int drm_display_mode_from_videomode(const struct videomode *vm,
+					   struct drm_display_mode *dmode);
+
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
 extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 5/7] fbmon: add of_videomode helpers
From: Steffen Trumtrar @ 2012-12-18 16:57 UTC (permalink / raw)
  To: devicestree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355849873-8051-1-git-send-email-s.trumtrar@pengutronix.de>

Add helper to get fb_videomode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/fbmon.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    4 ++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 17ce135..94ad0f7 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <video/of_videomode.h>
 #include <video/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
@@ -1425,6 +1426,47 @@ int fb_videomode_from_videomode(const struct videomode *vm,
 EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(const struct fb_videomode *m)
+{
+	pr_debug("fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u %u\n",
+		 m->xres, m->yres, m->refresh, m->pixclock, m->left_margin,
+		 m->right_margin, m->upper_margin, m->lower_margin,
+		 m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+			int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	fb_videomode_from_videomode(&vm, fb);
+
+	pr_debug("%s: got %dx%d display mode from %s\n",
+		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+	dump_fb_videomode(fb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 100a176..58b9860 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -20,6 +20,7 @@ struct fb_info;
 struct device;
 struct file;
 struct videomode;
+struct device_node;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF	1
@@ -715,6 +716,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int of_get_fb_videomode(struct device_node *np,
+			       struct fb_videomode *fb,
+			       int index);
 extern int fb_videomode_from_videomode(const struct videomode *vm,
 				       struct fb_videomode *fbmode);
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 4/7] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2012-12-18 16:57 UTC (permalink / raw)
  To: devicestree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355849873-8051-1-git-send-email-s.trumtrar@pengutronix.de>

Add a function to convert from the generic videomode to a fb_videomode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    4 ++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..17ce135 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <video/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
@@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
 	kfree(timings);
 	return err;
 }
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(const struct videomode *vm,
+				struct fb_videomode *fbmode)
+{
+	unsigned int htotal, vtotal;
+
+	fbmode->xres = vm->hactive;
+	fbmode->left_margin = vm->hback_porch;
+	fbmode->right_margin = vm->hfront_porch;
+	fbmode->hsync_len = vm->hsync_len;
+
+	fbmode->yres = vm->vactive;
+	fbmode->upper_margin = vm->vback_porch;
+	fbmode->lower_margin = vm->vfront_porch;
+	fbmode->vsync_len = vm->vsync_len;
+
+	/* prevent division by zero in KHZ2PICOS macro */
+	fbmode->pixclock = vm->pixelclock ?
+			KHZ2PICOS(vm->pixelclock / 1000) : 0;
+
+	fbmode->sync = 0;
+	fbmode->vmode = 0;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+		fbmode->vmode |= FB_VMODE_INTERLACED;
+	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+		fbmode->vmode |= FB_VMODE_DOUBLE;
+	fbmode->flag = 0;
+
+	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+		 vm->hsync_len;
+	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+		 vm->vsync_len;
+	/* prevent division by zero */
+	if (htotal && vtotal) {
+		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
+	/* a mode must have htotal and vtotal != 0 or it is invalid */
+	} else {
+		fbmode->refresh = 0;
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c7a9571..100a176 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -19,6 +19,7 @@ struct vm_area_struct;
 struct fb_info;
 struct device;
 struct file;
+struct videomode;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF	1
@@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int fb_videomode_from_videomode(const struct videomode *vm,
+				       struct fb_videomode *fbmode);
+
 /* drivers/video/modedb.c */
 #define VESA_MODEDB_SIZE 34
 extern void fb_var_to_videomode(struct fb_videomode *mode,
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 3/7] video: add of helper for display timings/videomode
From: Steffen Trumtrar @ 2012-12-18 16:57 UTC (permalink / raw)
  To: devicestree-discuss
  Cc: Steffen Trumtrar, Philipp Zabel, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, kernel, Florian Tobias Schandinat, David Airlie,
	Rob Clark, Leela Krishna Amudala
In-Reply-To: <1355849873-8051-1-git-send-email-s.trumtrar@pengutronix.de>

This adds support for reading display timings from DT into a struct
display_timings. The of_display_timing implementation supports multiple
subnodes. All children are read into an array, that can be queried.

If no native mode is specified, the first subnode will be used.

For cases where the graphics driver knows there can be only one
mode description or where the driver only supports one mode, a helper
function of_get_videomode is added, that gets a struct videomode from DT.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
 drivers/video/Kconfig                              |   15 ++
 drivers/video/Makefile                             |    2 +
 drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   54 +++++
 include/video/of_display_timing.h                  |   20 ++
 include/video/of_videomode.h                       |   18 ++
 7 files changed, 457 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 include/video/of_display_timing.h
 create mode 100644 include/video/of_videomode.h

diff --git a/Documentation/devicetree/bindings/video/display-timing.txt b/Documentation/devicetree/bindings/video/display-timing.txt
new file mode 100644
index 0000000..1500385
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timing.txt
@@ -0,0 +1,109 @@
+display-timing bindings
+===========+
+display-timings node
+--------------------
+
+required properties:
+ - none
+
+optional properties:
+ - native-mode: The native mode for the display, in case multiple modes are
+		provided. When omitted, assume the first node is the native.
+
+timing subnode
+--------------
+
+required properties:
+ - hactive, vactive: display resolution
+ - hfront-porch, hback-porch, hsync-len: horizontal display timing parameters
+   in pixels
+   vfront-porch, vback-porch, vsync-len: vertical display timing parameters in
+   lines
+ - clock-frequency: display clock in Hz
+
+optional properties:
+ - hsync-active: hsync pulse is active low/high/ignored
+ - vsync-active: vsync pulse is active low/high/ignored
+ - de-active: data-enable pulse is active low/high/ignored
+ - pixelclk-active: with
+			- active high = drive pixel data on rising edge/
+					sample data on falling edge
+			- active low  = drive pixel data on falling edge/
+					sample data on rising edge
+			- ignored     = ignored
+ - interlaced (bool): boolean to enable interlaced mode
+ - doublescan (bool): boolean to enable doublescan mode
+
+All the optional properties that are not bool follow the following logic:
+    <1>: high active
+    <0>: low active
+    omitted: not used on hardware
+
+There are different ways of describing the capabilities of a display. The
+devicetree representation corresponds to the one commonly found in datasheets
+for displays. If a display supports multiple signal timings, the native-mode
+can be specified.
+
+The parameters are defined as:
+
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vback_porch                 |          |       |
+  |          |        ↓                            |          |       |
+  +----------#######################################----------+-------+
+  |          #        ↑                            #          |       |
+  |          #        |                            #          |       |
+  |  hback   #        |                            #  hfront  | hsync |
+  |   porch  #        |       hactive              #  porch   |  len  |
+  |<-------->#<-------+--------------------------->#<-------->|<----->|
+  |          #        |                            #          |       |
+  |          #        |vactive                     #          |       |
+  |          #        |                            #          |       |
+  |          #        ↓                            #          |       |
+  +----------#######################################----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vfront_porch                |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vsync_len                   |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+
+Example:
+
+	display-timings {
+		native-mode = <&timing0>;
+		timing0: 1080p24 {
+			/* 1920x1080p24 */
+			clock-frequency = <52000000>;
+			hactive = <1920>;
+			vactive = <1080>;
+			hfront-porch = <25>;
+			hback-porch = <25>;
+			hsync-len = <25>;
+			vback-porch = <2>;
+			vfront-porch = <2>;
+			vsync-len = <2>;
+			hsync-active = <1>;
+		};
+	};
+
+Every required property also supports the use of ranges, so the commonly used
+datasheet description with minimum, typical and maximum values can be used.
+
+Example:
+
+	timing1: timing {
+		/* 1920x1080p24 */
+		clock-frequency = <148500000>;
+		hactive = <1920>;
+		vactive = <1080>;
+		hsync-len = <0 44 60>;
+		hfront-porch = <80 88 95>;
+		hback-porch = <100 148 160>;
+		vfront-porch = <0 4 6>;
+		vback-porch = <0 36 50>;
+		vsync-len = <0 5 6>;
+	};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2a23b18..c000f5a 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -39,6 +39,21 @@ config DISPLAY_TIMING
 config VIDEOMODE
        bool
 
+config OF_DISPLAY_TIMING
+	bool "Enable device tree display timing support"
+	depends on OF
+	select DISPLAY_TIMING
+	help
+	  helper to parse display timings from the devicetree
+
+config OF_VIDEOMODE
+	bool "Enable device tree videomode support"
+	depends on OF
+	select VIDEOMODE
+	select OF_DISPLAY_TIMING
+	help
+	  helper to get videomodes from the devicetree
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index fc30439..b936b00 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -168,4 +168,6 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
 obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o
 obj-$(CONFIG_VIDEOMODE) += videomode.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
new file mode 100644
index 0000000..13ecd98
--- /dev/null
+++ b/drivers/video/of_display_timing.c
@@ -0,0 +1,239 @@
+/*
+ * OF helpers for parsing display timings
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/export.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+
+/**
+ * parse_timing_property - parse timing_entry from device_node
+ * @np: device_node with the property
+ * @name: name of the property
+ * @result: will be set to the return value
+ *
+ * DESCRIPTION:
+ * Every display_timing can be specified with either just the typical value or
+ * a range consisting of min/typ/max. This function helps handling this
+ **/
+static int parse_timing_property(struct device_node *np, const char *name,
+			  struct timing_entry *result)
+{
+	struct property *prop;
+	int length, cells, ret;
+
+	prop = of_find_property(np, name, &length);
+	if (!prop) {
+		pr_err("%s: could not find property %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	cells = length / sizeof(u32);
+	if (cells = 1) {
+		ret = of_property_read_u32(np, name, &result->typ);
+		result->min = result->typ;
+		result->max = result->typ;
+	} else if (cells = 3) {
+		ret = of_property_read_u32_array(np, name, &result->min, cells);
+	} else {
+		pr_err("%s: illegal timing specification in %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+/**
+ * of_get_display_timing - parse display_timing entry from device_node
+ * @np: device_node with the properties
+ **/
+static struct display_timing *of_get_display_timing(struct device_node *np)
+{
+	struct display_timing *dt;
+	u32 val = 0;
+	int ret = 0;
+
+	dt = kzalloc(sizeof(*dt), GFP_KERNEL);
+	if (!dt) {
+		pr_err("%s: could not allocate display_timing struct\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	ret |= parse_timing_property(np, "hback-porch", &dt->hback_porch);
+	ret |= parse_timing_property(np, "hfront-porch", &dt->hfront_porch);
+	ret |= parse_timing_property(np, "hactive", &dt->hactive);
+	ret |= parse_timing_property(np, "hsync-len", &dt->hsync_len);
+	ret |= parse_timing_property(np, "vback-porch", &dt->vback_porch);
+	ret |= parse_timing_property(np, "vfront-porch", &dt->vfront_porch);
+	ret |= parse_timing_property(np, "vactive", &dt->vactive);
+	ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
+	ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
+
+	dt->dmt_flags = 0;
+	dt->data_flags = 0;
+	if (!of_property_read_u32(np, "vsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_VSYNC_HIGH :
+				VESA_DMT_VSYNC_LOW;
+	if (!of_property_read_u32(np, "hsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_HSYNC_HIGH :
+				VESA_DMT_HSYNC_LOW;
+	if (!of_property_read_u32(np, "de-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_DE_HIGH :
+				DISPLAY_FLAGS_DE_LOW;
+	if (!of_property_read_u32(np, "pixelclk-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_PIXDATA_POSEDGE :
+				DISPLAY_FLAGS_PIXDATA_NEGEDGE;
+
+	if (of_property_read_bool(np, "interlaced"))
+		dt->data_flags |= DISPLAY_FLAGS_INTERLACED;
+	if (of_property_read_bool(np, "doublescan"))
+		dt->data_flags |= DISPLAY_FLAGS_DOUBLESCAN;
+
+	if (ret) {
+		pr_err("%s: error reading timing properties\n",
+			of_node_full_name(np));
+		kfree(dt);
+		return NULL;
+	}
+
+	return dt;
+}
+
+/**
+ * of_get_display_timings - parse all display_timing entries from a device_node
+ * @np: device_node with the subnodes
+ **/
+struct display_timings *of_get_display_timings(struct device_node *np)
+{
+	struct device_node *timings_np;
+	struct device_node *entry;
+	struct device_node *native_mode;
+	struct display_timings *disp;
+
+	if (!np) {
+		pr_err("%s: no devicenode given\n", of_node_full_name(np));
+		return NULL;
+	}
+
+	timings_np = of_find_node_by_name(np, "display-timings");
+	if (!timings_np) {
+		pr_err("%s: could not find display-timings node\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	disp = kzalloc(sizeof(*disp), GFP_KERNEL);
+	if (!disp) {
+		pr_err("%s: could not allocate struct disp'\n",
+			of_node_full_name(np));
+		goto dispfail;
+	}
+
+	entry = of_parse_phandle(timings_np, "native-mode", 0);
+	/* assume first child as native mode if none provided */
+	if (!entry)
+		entry = of_get_next_child(np, NULL);
+	/* if there is no child, it is useless to go on */
+	if (!entry) {
+		pr_err("%s: no timing specifications given\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	pr_debug("%s: using %s as default timing\n",
+		of_node_full_name(np), entry->name);
+
+	native_mode = entry;
+
+	disp->num_timings = of_get_child_count(timings_np);
+	if (disp->num_timings = 0) {
+		/* should never happen, as entry was already found above */
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->timings = kzalloc(sizeof(struct display_timing *) *
+				disp->num_timings, GFP_KERNEL);
+	if (!disp->timings) {
+		pr_err("%s: could not allocate timings array\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->num_timings = 0;
+	disp->native_mode = 0;
+
+	for_each_child_of_node(timings_np, entry) {
+		struct display_timing *dt;
+
+		dt = of_get_display_timing(entry);
+		if (!dt) {
+			/*
+			 * to not encourage wrong devicetrees, fail in case of
+			 * an error
+			 */
+			pr_err("%s: error in timing %d\n",
+				of_node_full_name(np), disp->num_timings + 1);
+			goto timingfail;
+		}
+
+		if (native_mode = entry)
+			disp->native_mode = disp->num_timings;
+
+		disp->timings[disp->num_timings] = dt;
+		disp->num_timings++;
+	}
+	of_node_put(timings_np);
+	/*
+	 * native_mode points to the device_node returned by of_parse_phandle
+	 * therefore call of_node_put on it
+	 */
+	of_node_put(native_mode);
+
+	pr_debug("%s: got %d timings. Using timing #%d as default\n",
+		of_node_full_name(np), disp->num_timings,
+		disp->native_mode + 1);
+
+	return disp;
+
+timingfail:
+	if (native_mode)
+		of_node_put(native_mode);
+	display_timings_release(disp);
+entryfail:
+	kfree(disp);
+dispfail:
+	of_node_put(timings_np);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timings);
+
+/**
+ * of_display_timings_exist - check if a display-timings node is provided
+ * @np: device_node with the timing
+ **/
+int of_display_timings_exist(struct device_node *np)
+{
+	struct device_node *timings_np;
+
+	if (!np)
+		return -EINVAL;
+
+	timings_np = of_parse_phandle(np, "display-timings", 0);
+	if (!timings_np)
+		return -EINVAL;
+
+	of_node_put(timings_np);
+	return 1;
+}
+EXPORT_SYMBOL_GPL(of_display_timings_exist);
diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c
new file mode 100644
index 0000000..5b8066c
--- /dev/null
+++ b/drivers/video/of_videomode.c
@@ -0,0 +1,54 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/of.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+/**
+ * of_get_videomode - get the videomode #<index> from devicetree
+ * @np - devicenode with the display_timings
+ * @vm - set to return value
+ * @index - index into list of display_timings
+ *	    (Set this to OF_USE_NATIVE_MODE to use whatever mode is
+ *	     specified as native mode in the DT.)
+ *
+ * DESCRIPTION:
+ * Get a list of all display timings and put the one
+ * specified by index into *vm. This function should only be used, if
+ * only one videomode is to be retrieved. A driver that needs to work
+ * with multiple/all videomodes should work with
+ * of_get_display_timings instead.
+ **/
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index)
+{
+	struct display_timings *disp;
+	int ret;
+
+	disp = of_get_display_timings(np);
+	if (!disp) {
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		return -EINVAL;
+	}
+
+	if (index = OF_USE_NATIVE_MODE)
+		index = disp->native_mode;
+
+	ret = videomode_from_timing(disp, vm, index);
+	if (ret)
+		return ret;
+
+	display_timings_release(disp);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
diff --git a/include/video/of_display_timing.h b/include/video/of_display_timing.h
new file mode 100644
index 0000000..8016eb7
--- /dev/null
+++ b/include/video/of_display_timing.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * display timings of helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMING_H
+#define __LINUX_OF_DISPLAY_TIMING_H
+
+struct device_node;
+struct display_timings;
+
+#define OF_USE_NATIVE_MODE -1
+
+struct display_timings *of_get_display_timings(struct device_node *np);
+int of_display_timings_exist(struct device_node *np);
+
+#endif
diff --git a/include/video/of_videomode.h b/include/video/of_videomode.h
new file mode 100644
index 0000000..a07efcc
--- /dev/null
+++ b/include/video/of_videomode.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * videomode of-helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+struct device_node;
+struct videomode;
+
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index);
+
+#endif /* __LINUX_OF_VIDEOMODE_H */
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 2/7] video: add display_timing and videomode
From: Steffen Trumtrar @ 2012-12-18 16:57 UTC (permalink / raw)
  To: devicestree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355849873-8051-1-git-send-email-s.trumtrar@pengutronix.de>

Add display_timing structure and the according helper functions. This allows
the description of a display via its supported timing parameters.

Also, add helper functions to convert from display timings to a generic videomode
structure.

The struct display_timing specifies all needed parameters to describe the signal
properties of a display in one mode. This includes
    - ranges for signals that may have min-, max- and typical values
    - single integers for signals that can be on, off or are ignored
    - booleans for signals that are either on or off

As a display may support multiple modes like this, a struct display_timings is
added, that holds all given struct display_timing pointers and declares the
native mode of the display.

Although a display may state that a signal can be in a range, it is driven with
fixed values that indicate a videomode. Therefore graphic drivers don't need all
the information of struct display_timing, but would generate a videomode from
the given set of supported signal timings and work with that.

The video subsystems all define their own structs that describe a mode and work
with that (e.g. fb_videomode or drm_display_mode). To slowly replace all those
various structures and allow code reuse across those subsystems, add struct
videomode as a generic description.

This patch only includes the most basic fields in struct videomode. All missing
fields that are needed to have a really generic video mode description can be
added at a later stage.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/Kconfig          |    6 ++
 drivers/video/Makefile         |    2 +
 drivers/video/display_timing.c |   24 ++++++++
 drivers/video/videomode.c      |   39 +++++++++++++
 include/video/display_timing.h |  124 ++++++++++++++++++++++++++++++++++++++++
 include/video/videomode.h      |   48 ++++++++++++++++
 6 files changed, 243 insertions(+)
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/video/display_timing.h
 create mode 100644 include/video/videomode.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d08d799..2a23b18 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL
 	  This framework adds support for low-level control of the video 
 	  output switch.
 
+config DISPLAY_TIMING
+       bool
+
+config VIDEOMODE
+       bool
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 23e948e..fc30439 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -167,3 +167,5 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 0000000..5e1822c
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+
+void display_timings_release(struct display_timings *disp)
+{
+	if (disp->timings) {
+		unsigned int i;
+
+		for (i = 0; i < disp->num_timings; i++)
+			kfree(disp->timings[i]);
+		kfree(disp->timings);
+	}
+	kfree(disp);
+}
+EXPORT_SYMBOL_GPL(display_timings_release);
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 0000000..21c47a2
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,39 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <video/display_timing.h>
+#include <video/videomode.h>
+
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index)
+{
+	struct display_timing *dt;
+
+	dt = display_timings_get(disp, index);
+	if (!dt)
+		return -EINVAL;
+
+	vm->pixelclock = display_timing_get_value(&dt->pixelclock, TE_TYP);
+	vm->hactive = display_timing_get_value(&dt->hactive, TE_TYP);
+	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, TE_TYP);
+	vm->hback_porch = display_timing_get_value(&dt->hback_porch, TE_TYP);
+	vm->hsync_len = display_timing_get_value(&dt->hsync_len, TE_TYP);
+
+	vm->vactive = display_timing_get_value(&dt->vactive, TE_TYP);
+	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, TE_TYP);
+	vm->vback_porch = display_timing_get_value(&dt->vback_porch, TE_TYP);
+	vm->vsync_len = display_timing_get_value(&dt->vsync_len, TE_TYP);
+
+	vm->dmt_flags = dt->dmt_flags;
+	vm->data_flags = dt->data_flags;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/video/display_timing.h b/include/video/display_timing.h
new file mode 100644
index 0000000..71e9a38
--- /dev/null
+++ b/include/video/display_timing.h
@@ -0,0 +1,124 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_DISPLAY_TIMING_H
+#define __LINUX_DISPLAY_TIMING_H
+
+#include <linux/bitops.h>
+#include <linux/types.h>
+
+/* VESA display monitor timing parameters */
+#define VESA_DMT_HSYNC_LOW		BIT(0)
+#define VESA_DMT_HSYNC_HIGH		BIT(1)
+#define VESA_DMT_VSYNC_LOW		BIT(2)
+#define VESA_DMT_VSYNC_HIGH		BIT(3)
+
+/* display specific flags */
+#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
+#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
+#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
+#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
+#define DISPLAY_FLAGS_INTERLACED	BIT(4)
+#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
+
+/*
+ * A single signal can be specified via a range of minimal and maximal values
+ * with a typical value, that lies somewhere inbetween.
+ */
+struct timing_entry {
+	u32 min;
+	u32 typ;
+	u32 max;
+};
+
+enum timing_entry_index {
+	TE_MIN = 0,
+	TE_TYP = 1,
+	TE_MAX = 2,
+};
+
+/*
+ * Single "mode" entry. This describes one set of signal timings a display can
+ * have in one setting. This struct can later be converted to struct videomode
+ * (see include/video/videomode.h). As each timing_entry can be defined as a
+ * range, one struct display_timing may become multiple struct videomodes.
+ *
+ * Example: hsync active high, vsync active low
+ *
+ *				    Active Video
+ * Video  ______________________XXXXXXXXXXXXXXXXXXXXXX_____________________
+ *	  |<- sync ->|<- back ->|<----- active ----->|<- front ->|<- sync..
+ *	  |	     |	 porch  |		     |	 porch	 |
+ *
+ * HSync _|¯¯¯¯¯¯¯¯¯¯|___________________________________________|¯¯¯¯¯¯¯¯¯
+ *
+ * VSync ¯|__________|¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯|_________
+ */
+struct display_timing {
+	struct timing_entry pixelclock;
+
+	struct timing_entry hactive;		/* hor. active video */
+	struct timing_entry hfront_porch;	/* hor. front porch */
+	struct timing_entry hback_porch;	/* hor. back porch */
+	struct timing_entry hsync_len;		/* hor. sync len */
+
+	struct timing_entry vactive;		/* ver. active video */
+	struct timing_entry vfront_porch;	/* ver. front porch */
+	struct timing_entry vback_porch;	/* ver. back porch */
+	struct timing_entry vsync_len;		/* ver. sync len */
+
+	unsigned int dmt_flags;			/* VESA DMT flags */
+	unsigned int data_flags;		/* video data flags */
+};
+
+/*
+ * This describes all timing settings a display provides.
+ * The native_mode is the default setting for this display.
+ * Drivers that can handle multiple videomodes should work with this struct and
+ * convert each entry to the desired end result.
+ */
+struct display_timings {
+	unsigned int num_timings;
+	unsigned int native_mode;
+
+	struct display_timing **timings;
+};
+
+/* get value specified by index from struct timing_entry */
+static inline u32 display_timing_get_value(const struct timing_entry *te,
+					   enum timing_entry_index index)
+{
+	switch (index) {
+	case TE_MIN:
+		return te->min;
+		break;
+	case TE_TYP:
+		return te->typ;
+		break;
+	case TE_MAX:
+		return te->max;
+		break;
+	default:
+		return te->typ;
+	}
+}
+
+/* get one entry from struct display_timings */
+static inline struct display_timing *display_timings_get(const struct
+							 display_timings *disp,
+							 unsigned int index)
+{
+	if (disp->num_timings > index)
+		return disp->timings[index];
+	else
+		return NULL;
+}
+
+void display_timings_release(struct display_timings *disp);
+
+#endif
diff --git a/include/video/videomode.h b/include/video/videomode.h
new file mode 100644
index 0000000..a421562
--- /dev/null
+++ b/include/video/videomode.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * generic videomode description
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_VIDEOMODE_H
+#define __LINUX_VIDEOMODE_H
+
+#include <linux/types.h>
+#include <video/display_timing.h>
+
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+	unsigned long pixelclock;	/* pixelclock in Hz */
+
+	u32 hactive;
+	u32 hfront_porch;
+	u32 hback_porch;
+	u32 hsync_len;
+
+	u32 vactive;
+	u32 vfront_porch;
+	u32 vback_porch;
+	u32 vsync_len;
+
+	unsigned int dmt_flags;	/* VESA DMT flags */
+	unsigned int data_flags; /* video data flags */
+};
+
+/**
+ * videomode_from_timing - convert display timing to videomode
+ * @disp: structure with all possible timing entries
+ * @vm: return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function converts a struct display_timing to a struct videomode.
+ */
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index);
+
+#endif
-- 
1.7.10.4


^ permalink raw reply related

* [PATCHv16 1/7] viafb: rename display_timing to via_display_timing
From: Steffen Trumtrar @ 2012-12-18 16:57 UTC (permalink / raw)
  To: devicestree-discuss
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren, kernel,
	Florian Tobias Schandinat, David Airlie, Rob Clark,
	Leela Krishna Amudala
In-Reply-To: <1355849873-8051-1-git-send-email-s.trumtrar@pengutronix.de>

The struct display_timing is specific to the via subsystem. The naming leads to
collisions with the new struct display_timing, which is supposed to be a shared
struct between different subsystems.
To clean this up, prepend the existing struct with the subsystem it is specific
to.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/via/hw.c              |    6 +++---
 drivers/video/via/hw.h              |    2 +-
 drivers/video/via/lcd.c             |    2 +-
 drivers/video/via/share.h           |    2 +-
 drivers/video/via/via_modesetting.c |    8 ++++----
 drivers/video/via/via_modesetting.h |    6 +++---
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 898590d..5563c67 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -1467,10 +1467,10 @@ void viafb_set_vclock(u32 clk, int set_iga)
 	via_write_misc_reg_mask(0x0C, 0x0C); /* select external clock */
 }
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres)
 {
-	struct display_timing timing;
+	struct via_display_timing timing;
 	u16 dx = (var->xres - cxres) / 2, dy = (var->yres - cyres) / 2;
 
 	timing.hor_addr = cxres;
@@ -1491,7 +1491,7 @@ struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga)
 {
-	struct display_timing crt_reg = var_to_timing(var,
+	struct via_display_timing crt_reg = var_to_timing(var,
 		cxres ? cxres : var->xres, cyres ? cyres : var->yres);
 
 	if (iga = IGA1)
diff --git a/drivers/video/via/hw.h b/drivers/video/via/hw.h
index 6be243c..c3f2572 100644
--- a/drivers/video/via/hw.h
+++ b/drivers/video/via/hw.h
@@ -637,7 +637,7 @@ extern int viafb_LCD_ON;
 extern int viafb_DVI_ON;
 extern int viafb_hotplug;
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres);
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga);
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 1650379..022b0df 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -549,7 +549,7 @@ void viafb_lcd_set_mode(const struct fb_var_screeninfo *var, u16 cxres,
 	int panel_hres = plvds_setting_info->lcd_panel_hres;
 	int panel_vres = plvds_setting_info->lcd_panel_vres;
 	u32 clock;
-	struct display_timing timing;
+	struct via_display_timing timing;
 	struct fb_var_screeninfo panel_var;
 	const struct fb_videomode *mode_crt_table, *panel_crt_table;
 
diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
index 3158dfc..65c65c6 100644
--- a/drivers/video/via/share.h
+++ b/drivers/video/via/share.h
@@ -319,7 +319,7 @@ struct crt_mode_table {
 	int refresh_rate;
 	int h_sync_polarity;
 	int v_sync_polarity;
-	struct display_timing crtc;
+	struct via_display_timing crtc;
 };
 
 struct io_reg {
diff --git a/drivers/video/via/via_modesetting.c b/drivers/video/via/via_modesetting.c
index 0e431ae..0b414b0 100644
--- a/drivers/video/via/via_modesetting.c
+++ b/drivers/video/via/via_modesetting.c
@@ -30,9 +30,9 @@
 #include "debug.h"
 
 
-void via_set_primary_timing(const struct display_timing *timing)
+void via_set_primary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total / 8 - 5;
 	raw.hor_addr = timing->hor_addr / 8 - 1;
@@ -88,9 +88,9 @@ void via_set_primary_timing(const struct display_timing *timing)
 	via_write_reg_mask(VIACR, 0x17, 0x80, 0x80);
 }
 
-void via_set_secondary_timing(const struct display_timing *timing)
+void via_set_secondary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total - 1;
 	raw.hor_addr = timing->hor_addr - 1;
diff --git a/drivers/video/via/via_modesetting.h b/drivers/video/via/via_modesetting.h
index 06e09fe..f6a6503 100644
--- a/drivers/video/via/via_modesetting.h
+++ b/drivers/video/via/via_modesetting.h
@@ -33,7 +33,7 @@
 #define VIA_PITCH_MAX	0x3FF8
 
 
-struct display_timing {
+struct via_display_timing {
 	u16 hor_total;
 	u16 hor_addr;
 	u16 hor_blank_start;
@@ -49,8 +49,8 @@ struct display_timing {
 };
 
 
-void via_set_primary_timing(const struct display_timing *timing);
-void via_set_secondary_timing(const struct display_timing *timing);
+void via_set_primary_timing(const struct via_display_timing *timing);
+void via_set_secondary_timing(const struct via_display_timing *timing);
 void via_set_primary_address(u32 addr);
 void via_set_secondary_address(u32 addr);
 void via_set_primary_pitch(u32 pitch);
-- 
1.7.10.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox