Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* How to set fops in "struct platform_pwm_backlight_data"?
From: Mark Zhang @ 2013-10-17  6:49 UTC (permalink / raw)
  To: thierry.reding, rpurdie, jg1.han,
	Jean-Christophe PLAGNIOL-VILLARD, tomi.valkeinen
  Cc: linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

Hi,

This is the first time I send mail to linux-pwm, I didn't read through
the mails in this list, so if somebody already asked this question, I'm
sorry about that.

I wanna set some fops in "struct platform_pwm_backlight_data". But the
currrent probe function in pwm_bl.c says:

-------
if (!data) {
	ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
	if (ret < 0) {
		dev_err(&pdev->dev, "failed to find platform data\n");
		return ret;
	}

	data = &defdata;
}
-------

This looks like if we set the platform data for pwm backlight device,
"pwm_backlight_parse_dt" will never have a chance to be called, which
means the stuffs I defined in backlight DT node will be ignored.

If I don't set the platform data for pwm backlight device, according to
the pwm_backlight_probe, I will never have a chance to set some fops
which I need(like "notify", "check_fb"...).

So, what I suppose to do now? Maybe there is a way to set function
pointers in DT?

Mark

^ permalink raw reply

* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Thierry Reding @ 2013-10-17  7:14 UTC (permalink / raw)
  To: Mark Zhang
  Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
	tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <525F8895.5010806@gmail.com>

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

On Thu, Oct 17, 2013 at 02:49:57PM +0800, Mark Zhang wrote:
> Hi,
> 
> This is the first time I send mail to linux-pwm, I didn't read through
> the mails in this list, so if somebody already asked this question, I'm
> sorry about that.
> 
> I wanna set some fops in "struct platform_pwm_backlight_data". But the
> currrent probe function in pwm_bl.c says:
> 
> -------
> if (!data) {
> 	ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
> 	if (ret < 0) {
> 		dev_err(&pdev->dev, "failed to find platform data\n");
> 		return ret;
> 	}
> 
> 	data = &defdata;
> }
> -------
> 
> This looks like if we set the platform data for pwm backlight device,
> "pwm_backlight_parse_dt" will never have a chance to be called, which
> means the stuffs I defined in backlight DT node will be ignored.
> 
> If I don't set the platform data for pwm backlight device, according to
> the pwm_backlight_probe, I will never have a chance to set some fops
> which I need(like "notify", "check_fb"...).
> 
> So, what I suppose to do now? Maybe there is a way to set function
> pointers in DT?

Perhaps you could describe in more detail what you need the functions
for.

Generally you're not supposed to mix DT and platform data. Without more
info that's about all I can say.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Andrzej Hajda @ 2013-10-17  7:48 UTC (permalink / raw)
  To: Tomi Valkeinen, Laurent Pinchart
  Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
	Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
	Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
	Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <52580EFF.2020401@ti.com>

Hi Tomi,

Sorry for delayed response.


On 10/11/2013 04:45 PM, Tomi Valkeinen wrote:
> On 11/10/13 17:16, Andrzej Hajda wrote:
>
>> Picture size, content and format is the same on input and on output of DSI.
>> The same bits which enters DSI appears on the output. Internally bits
>> order can
>> be different but practically you are configuring DSI master and slave
>> with the same format.
>>
>> If you create DSI entity you will have to always set the same format and
>> size on DSI input, DSI output and encoder input.
>> If you skip creating DSI entity you loose nothing, and you do not need
>> to take care of it.
> Well, this is really a different question from the bus problem. But
> nothing says the DSI master cannot change the format or even size. For
> sure it can change the video timings. The DSI master could even take two
> parallel inputs, and combine them into one DSI output. You don't can't
> what all the possible pieces of hardware do =)
> If you have a bigger IP block that internally contains the DISPC and the
> DSI, then, yes, you can combine them into one display entity. I don't
> think that's correct, though. And if the DISPC and DSI are independent
> blocks, then especially I think there must be an entity for the DSI
> block, which will enable the powers, clocks, etc, when needed.
The main function of DSI is to transport pixels from one IP to another IP
and this function IMO should not be modeled by display entity.
"Power, clocks, etc" will be performed via control bus according to
panel demands.
If 'DSI chip' has additional functions for video processing they can
be modeled by CDF entity if it makes sense.
>>> Well, one point of the endpoints is also to allow "switching" of video
>>> devices.
>>>
>>> For example, I could have a board with a SoC's DSI output, connected to
>>> two DSI panels. There would be some kind of mux between, so that I can
>>> select which of the panels is actually connected to the SoC.
>>>
>>> Here the first panel could use 2 datalanes, the second one 4. Thus, the
>>> DSI master would have two endpoints, the other one using 2 and the other
>>> 4 datalanes.
>>>
>>> If we decide that kind of support is not needed, well, is there even
>>> need for the V4L2 endpoints in the DT data at all?
>> Hmm, both panels connected to one endpoint of dispc ?
>> The problem I see is which driver should handle panel switching,
>> but this is question about hardware design as well. If this is realized
>> by dispc I have told already the solution. If this is realized by other
>> device I do not see a problem to create corresponding CDF entity,
>> or maybe it can be handled by "Pipeline Controller" ???
> Well the switching could be automatic, when the panel power is enabled,
> the DSI mux is switched for that panel. It's not relevant.
>
> We still have two different endpoint configurations for the same
> DSI-master port. If that configuration is in the DSI-master's port node,
> not inside an endpoint data, then that can't be supported.
I am not sure if I understand it correctly. But it seems quite simple:
when panel starts/resumes it request DSI (via control bus) to fulfill
its configuration settings.
Of course there are some settings which are not panel dependent and those
should reside in DSI node.
>>>>> I agree that having DSI/DBI control and video separated would be
>>>>> elegant. But I'd like to hear what is the technical benefit of that? At
>>>>> least to me it's clearly more complex to separate them than to keep them
>>>>> together (to the extent that I don't yet see how it is even possible),
>>>>> so there must be a good reason for the separation. I don't understand
>>>>> that reason. What is it?
>>>> Roughly speaking it is a question where is the more convenient place to
>>>> put bunch
>>>> of opses, technically both solutions can be somehow implemented.
>>> Well, it's also about dividing a single physical bus into two separate
>>> interfaces to it. It sounds to me that it would be much more complex
>>> with locking. With a single API, we can just say "the caller handles
>>> locking". With two separate interfaces, there must be locking at the
>>> lower level.
>> We say then: callee handles locking :)
> Sure, but my point was that the caller handling the locking is much
> simpler than the callee handling locking. And the latter causes
> atomicity issues, as the other API could be invoked in between two calls
> for the first API.
>
>     
Could you describe such scenario?
> But note that I'm not saying we should not implement bus model just
> because it's more complex. We should go for bus model if it's better. I
> just want to bring up these complexities, which I feel are quite more
> difficult than with the simpler model.
>
>>>> Pros of mipi bus:
>>>> - no fake entity in CDF, with fake opses, I have to use similar entities
>>>> in MIPI-CSI
>>>> camera pipelines and it complicates life without any benefit(at least
>>>> from user side),
>>> You mean the DSI-master? I don't see how it's "fake", it's a video
>>> processing unit that has to be configured. Even if we forget the control
>>> side, and just think about plain video stream with DSI video mode,
>>> there's are things to configure with it.
>>>
>>> What kind of issues you have in the CSI side, then?
>> Not real issues, just needless calls to configure CSI entity pads,
>> with the same format and picture sizes as in camera.
> Well, the output of a component A is surely the same as the input of
> component B, if B receives the data from A. So that does sound useless.
> I don't do that kind of calls in my model.
>
>>>> - CDF models only video buses, control bus is a domain of Linux buses,
>>> Yes, but in this case the buses are the same. It makes me a bit nervous
>>> to have two separate ways (video and control) to use the same bus, in a
>>> case like video where timing is critical.
>>>
>>> So yes, we can consider video and control buses as "virtual" buses, and
>>> the actual transport is the DSI bus. Maybe it can be done. It just makes
>>> me a bit nervous =).
>>>
>>>> - less platform_bus abusing,
>>> Well, platform.txt says
>>>
>>> "This pseudo-bus
>>> is used to connect devices on busses with minimal infrastructure,
>>> like those used to integrate peripherals on many system-on-chip
>>> processors, or some "legacy" PC interconnects; as opposed to large
>>> formally specified ones like PCI or USB."
>>>
>>> I don't think DSI and DBI as platform bus is that far from the
>>> description. They are "simple", no probing point-to-point (in practice)
>>> buses. There's not much "bus" to speak of, just a point-to-point link.
>> Next section:
>>
>> Platform devices
>> ~~~~~~~~~~~~~~~~
>> Platform devices are devices that typically appear as autonomous
>> entities in the system. This includes legacy port-based devices and
>> host bridges to peripheral buses, and most controllers integrated
>> into system-on-chip platforms.  What they usually have in common
>> is direct addressing from a CPU bus.  Rarely, a platform_device will
>> be connected through a segment of some other kind of bus; but its
>> registers will still be directly addressable.
> Yep, "typically" and "rarely" =). I agree, it's not clear. I think there
> are things with DBI/DSI that clearly point to a platform device, but
> also the other way.
Just to be sure, we are talking here about DSI-slaves, ie. for example
about panels,
where direct accessing from CPU bus usually is not possible.

Andrzej
>>>> - better device tree topology (at least for common cases),
>>> Even if we use platform devices for DSI peripherals, we can have them
>>> described under the DSI master node.
>> Sorry, I meant rather Linux device tree topology, not DT.
> We can have the DSI peripheral platform devices as children of the
> DSI-master device.
>
>  Tomi
>
>


^ permalink raw reply

* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Tomi Valkeinen @ 2013-10-17  8:18 UTC (permalink / raw)
  To: Andrzej Hajda, Laurent Pinchart
  Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
	Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
	Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
	Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <525F9660.3010408@samsung.com>

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

On 17/10/13 10:48, Andrzej Hajda wrote:

> The main function of DSI is to transport pixels from one IP to another IP
> and this function IMO should not be modeled by display entity.
> "Power, clocks, etc" will be performed via control bus according to
> panel demands.
> If 'DSI chip' has additional functions for video processing they can
> be modeled by CDF entity if it makes sense.

Now I don't follow. What do you mean with "display entity" and with "CDF
entity"? Are they the same?

Let me try to clarify my point:

On OMAP SoC we have a DSI encoder, which takes input from the display
controller in parallel RGB format, and outputs DSI.

Then there are external encoders that take MIPI DPI as input, and output
DSI.

The only difference with the above two components is that the first one
is embedded into the SoC. I see no reason to represent them in different
ways (i.e. as you suggested, not representing the SoC's DSI at all).

Also, if you use DSI burst mode, you will have to have different video
timings in the DSI encoder's input and output. And depending on the
buffering of the DSI encoder, you could have different timings in any case.

Furthermore, both components could have extra processing. I know the
external encoders sometimes do have features like scaling.

>> We still have two different endpoint configurations for the same
>> DSI-master port. If that configuration is in the DSI-master's port node,
>> not inside an endpoint data, then that can't be supported.
> I am not sure if I understand it correctly. But it seems quite simple:
> when panel starts/resumes it request DSI (via control bus) to fulfill
> its configuration settings.
> Of course there are some settings which are not panel dependent and those
> should reside in DSI node.

Exactly. And when the two panels require different non-panel-dependent
settings, how do you represent them in the DT data?

>>> We say then: callee handles locking :)
>> Sure, but my point was that the caller handling the locking is much
>> simpler than the callee handling locking. And the latter causes
>> atomicity issues, as the other API could be invoked in between two calls
>> for the first API.
>>
>>     
> Could you describe such scenario?

If we have two independent APIs, ctrl and video, that affect the same
underlying hardware, the DSI bus, we could have a scenario like this:

thread 1:

ctrl->op_foo();
ctrl->op_bar();

thread 2:

video->op_baz();

Even if all those ops do locking properly internally, the fact that
op_baz() can be called in between op_foo() and op_bar() may cause problems.

To avoid that issue with two APIs we'd need something like:

thread 1:

ctrl->lock();
ctrl->op_foo();
ctrl->op_bar();
ctrl->unlock();

thread 2:

video->lock();
video->op_baz();
video->unlock();

>>> Platform devices
>>> ~~~~~~~~~~~~~~~~
>>> Platform devices are devices that typically appear as autonomous
>>> entities in the system. This includes legacy port-based devices and
>>> host bridges to peripheral buses, and most controllers integrated
>>> into system-on-chip platforms.  What they usually have in common
>>> is direct addressing from a CPU bus.  Rarely, a platform_device will
>>> be connected through a segment of some other kind of bus; but its
>>> registers will still be directly addressable.
>> Yep, "typically" and "rarely" =). I agree, it's not clear. I think there
>> are things with DBI/DSI that clearly point to a platform device, but
>> also the other way.
> Just to be sure, we are talking here about DSI-slaves, ie. for example
> about panels,
> where direct accessing from CPU bus usually is not possible.

Yes. My point is that with DBI/DSI there's not much bus there (if a
normal bus would be PCI/USB/i2c etc), it's just a point to point link
without probing or a clearly specified setup sequence.

If DSI/DBI was used only for control, a linux bus would probably make
sense. But DSI/DBI is mainly a video transport channel, with the
control-part being "secondary".

And when considering that the video and control data are sent over the
same channel (i.e. there's no separate, independent ctrl channel), and
the strict timing restrictions with video, my gut feeling is just that
all the extra complexity brought with separating the control to a
separate bus is not worth it.

 Tomi



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

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Dave Airlie @ 2013-10-17  8:28 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Thierry Reding, Laurent Pinchart, Tomi Valkeinen, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <1695169.rzbDl9PeRX@avalon>

>
> My biggest concern here is that this will not be compatible with the CDF DT
> bindings. They're not complete yet, but they will require connections between
> entities to be described in DT, in a way very similar (or actually identical)
> to the V4L2 DT bindings, documented in
> Documentation/devicetree/bindings/media/video-interfaces.txt. Could you have a
> look at that ? Please ignore all optional properties beside remote-endpoint,
> as they're V4L2 specific.
>
> I also plan to specify video bus parameters in DT for CDF, but this hasn't
> been finalized yet.
>

While I understand this, I don't see why CDF can't enhance these
bindings if it has
requirements > than they have without disturbing the panel ones,

is DT really that inflexible?

It seems that have a simple description for basic panels like Thierry wants
to support, that can be enhanced for the other cases in the future should
suffice, I really don't like blocking stuff that makes things work on the chance
of something that isn't upstream yet, its sets a bad precedent, its also breaks
the perfect is the enemy of good rule

Dave.

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Tomi Valkeinen @ 2013-10-17  8:49 UTC (permalink / raw)
  To: Dave Airlie, Laurent Pinchart
  Cc: Thierry Reding, Laurent Pinchart, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <CAPM=9tzU36cwcM0pH0J_Vgc6UOj5MHdVNDvn3wpGNuihbMqdQQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

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

Hi,

On 17/10/13 11:28, Dave Airlie wrote:
>>
>> My biggest concern here is that this will not be compatible with the CDF DT
>> bindings. They're not complete yet, but they will require connections between
>> entities to be described in DT, in a way very similar (or actually identical)
>> to the V4L2 DT bindings, documented in
>> Documentation/devicetree/bindings/media/video-interfaces.txt. Could you have a
>> look at that ? Please ignore all optional properties beside remote-endpoint,
>> as they're V4L2 specific.
>>
>> I also plan to specify video bus parameters in DT for CDF, but this hasn't
>> been finalized yet.
>>
> 
> While I understand this, I don't see why CDF can't enhance these
> bindings if it has
> requirements > than they have without disturbing the panel ones,
> 
> is DT really that inflexible?
> 
> It seems that have a simple description for basic panels like Thierry wants
> to support, that can be enhanced for the other cases in the future should
> suffice, I really don't like blocking stuff that makes things work on the chance
> of something that isn't upstream yet, its sets a bad precedent, its also breaks
> the perfect is the enemy of good rule

Just my opinion, but yes, DT is that inflexible. DT bindings are like a
syscall. Once they are there, you need to support them forever. That
support can, of course, include some kind of DT data converters or such,
that try to convert old bindings to new bindings at kernel boot.

In practice even such converter may be enough, if some important piece
of data in the new bindings is missing, and this data was implicit in a
driver. In that case the conversion, or parts of it, must be done later,
in that specific driver.

Even that may be difficult, if the piece of data should actually be
handled by some other driver. In that case there's probably need for
some kind of global look-up table or such, so that the drivers can work
around the problem of missing information.

I've been working with DT bindings for OMAP display subsystem for
something like 1.5 years. Still not merged, as they are not perfect, and
I'm scared to push them in non-perfect form, as that could result in
some kind of DT-binding-converter-hell described above.

 Tomi



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

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-17  8:53 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Laurent Pinchart, Tomi Valkeinen, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <1695169.rzbDl9PeRX@avalon>

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

Adding the dri-devel mailing list on Cc, since this discussion is
outgrowing its original intent.

On Thu, Oct 17, 2013 at 02:47:52AM +0200, Laurent Pinchart wrote:
> Hi Thierry,
> 
> Thank you for the patch.
> 
> On Wednesday 16 October 2013 20:25:12 Thierry Reding wrote:
> > Add a driver for simple panels. Such panels can have a regulator that
> > provides the supply voltage and a separate GPIO to enable the panel.
> > Optionally the panels can have a backlight associated with them so it
> > can be enabled or disabled according to the panel's power management
> > mode.
> > 
> > Support is added for three panels: An AU Optronics 10.1" WSVGA, a
> > Chunghwa Picture Tubes 10.1" WXGA and a Panasonic 10.1 WUXGA TFT LCD
> > panel.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  .../devicetree/bindings/panel/auo,b101aw03.txt     |   7 +
> >  .../bindings/panel/chunghwa,claa101wb03.txt        |   7 +
> >  .../bindings/panel/panasonic,vvx10f004b00.txt      |   7 +
> >  .../devicetree/bindings/panel/simple-panel.txt     |  21 ++
> >  drivers/gpu/drm/Makefile                           |   1 +
> >  drivers/gpu/drm/panel/Kconfig                      |  13 +
> >  drivers/gpu/drm/panel/Makefile                     |   1 +
> >  drivers/gpu/drm/panel/panel-simple.c               | 335 ++++++++++++++++++
> >  8 files changed, 392 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> > create mode 100644
> > Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt create
> > mode 100644
> > Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt create
> > mode 100644 Documentation/devicetree/bindings/panel/simple-panel.txt create
> > mode 100644 drivers/gpu/drm/panel/Makefile
> >  create mode 100644 drivers/gpu/drm/panel/panel-simple.c
> > 
> > diff --git a/Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> > b/Documentation/devicetree/bindings/panel/auo,b101aw03.txt new file mode
> > 100644
> > index 0000000..72e088a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> > @@ -0,0 +1,7 @@
> > +AU Optronics Corporation 10.1" WSVGA TFT LCD panel
> > +
> > +Required properties:
> > +- compatible: should be "auo,b101aw03"
> > +
> > +This binding is compatible with the simple-panel binding, which is
> > specified +in simple-panel.txt in this directory.
> > diff --git
> > a/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
> > b/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt new file
> > mode 100644
> > index 0000000..0ab2c05
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
> > @@ -0,0 +1,7 @@
> > +Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel
> > +
> > +Required properties:
> > +- compatible: should be "chunghwa,claa101wb03"
> > +
> > +This binding is compatible with the simple-panel binding, which is
> > specified +in simple-panel.txt in this directory.
> > diff --git
> > a/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
> > b/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt new
> > file mode 100644
> > index 0000000..d328b03
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
> > @@ -0,0 +1,7 @@
> > +Panasonic Corporation 10.1" WUXGA TFT LCD panel
> > +
> > +Required properties:
> > +- compatible: should be "panasonic,vvx10f004b00"
> > +
> > +This binding is compatible with the simple-panel binding, which is
> > specified +in simple-panel.txt in this directory.
> > diff --git a/Documentation/devicetree/bindings/panel/simple-panel.txt
> > b/Documentation/devicetree/bindings/panel/simple-panel.txt new file mode
> > 100644
> > index 0000000..1341bbf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/panel/simple-panel.txt
> > @@ -0,0 +1,21 @@
> > +Simple display panel
> > +
> > +Required properties:
> > +- power-supply: regulator to provide the supply voltage
> > +
> > +Optional properties:
> > +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> > +- enable-gpios: GPIO pin to enable or disable the panel
> > +- backlight: phandle of the backlight device attached to the panel
> > +
> > +Example:
> > +
> > +	panel: panel {
> > +		compatible = "cptt,claa101wb01";
> > +		ddc-i2c-bus = <&panelddc>;
> > +
> > +		power-supply = <&vdd_pnl_reg>;
> > +		enable-gpios = <&gpio 90 0>;
> > +
> > +		backlight = <&backlight>;
> > +	};
> 
> My biggest concern here is that this will not be compatible with the CDF DT 
> bindings. They're not complete yet, but they will require connections between 
> entities to be described in DT, in a way very similar (or actually identical) 
> to the V4L2 DT bindings, documented in 
> Documentation/devicetree/bindings/media/video-interfaces.txt. Could you have a 
> look at that ? Please ignore all optional properties beside remote-endpoint, 
> as they're V4L2 specific.

Okay, so if I understand correctly, translating those bindings to panel
nodes would look somewhat like this:

	dc: display-controller {
		ports {
			port@0 {
				remote-endpoint = <&panel>;
			};
		};
	};

	panel: panel {
		ports {
			port@0 {
				remote-endpoint = <&dc>;
			};
		};
	};

The above leaves out any of the other, non-relevant properties. Does
that sound about right? That seems like an overly complex amount of data
to describe just a simple panel where the only connection between it and
the display hardware is the data bus and I was sort of expecting the CDF
to provide some sort of shortcut for setups where the connection is 1:1
with no means to perform any configuration of the bus.

> I also plan to specify video bus parameters in DT for CDF, but this hasn't 
> been finalized yet.

That effectively blocks any of the code that I've been working on until
CDF has been merged. It's been discussed for over a year now, and there
has even been significant pushback on using CDF in DRM, so even if CDF
is eventually merged, that will still leave DRM with no way to turn on
a simple panel.

I keep wondering why we absolutely must have compatibility between CDF
and this simple panel binding. DT content is supposed to concern itself
with the description of hardware only. What about the following isn't an
accurate description of the panel hardware?

	panel: panel {
		compatible = "cptt,claa101wb01";

		power-supply = <&vdd_pnl_reg>;
		enable-gpios = <&gpio 90 0>;

		backlight = <&backlight>;
	};

	dsi-controller {
		panel = <&panel>;
	};

Note how the above is also not incompatible with anything that the CDF
(to be) mandates, so it could easily be extended with any nodes or
properties that CDF needs to work properly without breaking backwards-
compatibility.

> > diff --git a/drivers/gpu/drm/panel/panel-simple.c
[...]
> > +/*
> > + * Copyright (C) 2013, NVIDIA Corporation.  All rights reserved.
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sub
> > license,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial portions
> > + * of the Software.
> 
> This contradicts MODULE_LICENSE("GPL v2") below.

True, I'll fix that up.

> > +/* TODO: convert to gpiod_*() API once it's been merged */
> > +#define GPIO_ACTIVE_LOW	(1 << 0)
> 
> Why can't you just #include <dt-bindings/gpio/gpio.h> ?

This is supposed to be something completely driver internal. Keeping it
here make that more explicit. It shouldn't matter much anyway, since by
now these patches have about 0 chance of making it into 3.13, so I'll
just convert them to gpiod.

> > +		backlight_update_status(p->backlight);
> > +	}
> > +
> > +	if (gpio_is_valid(p->enable_gpio)) {
> > +		if (p->enable_gpio_flags & GPIO_ACTIVE_LOW)
> > +			gpio_set_value(p->enable_gpio, 1);
> > +		else
> > +			gpio_set_value(p->enable_gpio, 0);
> > +	}
> > +
> > +	regulator_disable(p->supply);
> > +	p->enabled = false;
> > +}
> > +
> > +static void panel_simple_enable(struct drm_panel *panel)
> > +{
> > +	struct panel_simple *p = to_panel_simple(panel);
> > +	int err;
> > +
> > +	if (p->enabled)
> > +		return;
> > +
> > +	err = regulator_enable(p->supply);
> > +	if (err < 0)
> > +		dev_err(panel->dev, "failed to enable supply: %d\n", err);
> 
> Is that really a non-fatal error ? Shouldn't the enable operation return an 
> int ?

There's no way to propagate this in DRM, so why go through the trouble
of returning the error? Furthermore, there's nothing that the caller
could do to remedy the situation anyway.

> > +static const struct drm_display_mode auo_b101aw03_mode = {
> > +	.clock = 51450,
> > +	.hdisplay = 1024,
> > +	.hsync_start = 1024 + 156,
> > +	.hsync_end = 1024 + 156 + 8,
> > +	.htotal = 1024 + 156 + 8 + 156,
> > +	.vdisplay = 600,
> > +	.vsync_start = 600 + 16,
> > +	.vsync_end = 600 + 16 + 6,
> > +	.vtotal = 600 + 16 + 6 + 16,
> > +	.vrefresh = 60,
> > +};
> 
> Instead of hardcoding the modes in the driver, which would then require to be 
> updated for every new simple panel model (and we know there are lots of them), 
> why don't you specify the modes in the panel DT node ? The simple panel driver 
> would then become much more generic. It would also allow board designers to 
> tweak h/v sync timings depending on the system requirements.

Sigh... we keep second-guessing ourselves. Back at the time when power
sequences were designed (and NAKed at the last minute), we all decided
that the right thing to do would be to use specific compatible values
for individual panels, because that would allow us to encode the power
sequencing within the driver. And when we already have the panel model
encoded in the compatible value, we might just as well encode the mode
within the driver for that panel. Otherwise we'll have to keep adding
the same mode timings for every board that uses the same panel.

I do agree though that it might be useful to tweak the mode in case the
default one doesn't work. How about we provide a means to override the
mode encoded in the driver using one specified in the DT? That's
obviously a backwards-compatible change, so it could be added if or when
it becomes necessary.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-17  9:16 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Dave Airlie, Laurent Pinchart, Laurent Pinchart, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <525FA4B0.8060504-l0cyMroinI0@public.gmane.org>

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

On Thu, Oct 17, 2013 at 11:49:52AM +0300, Tomi Valkeinen wrote:
> Hi,
> 
> On 17/10/13 11:28, Dave Airlie wrote:
> >>
> >> My biggest concern here is that this will not be compatible with the CDF DT
> >> bindings. They're not complete yet, but they will require connections between
> >> entities to be described in DT, in a way very similar (or actually identical)
> >> to the V4L2 DT bindings, documented in
> >> Documentation/devicetree/bindings/media/video-interfaces.txt. Could you have a
> >> look at that ? Please ignore all optional properties beside remote-endpoint,
> >> as they're V4L2 specific.
> >>
> >> I also plan to specify video bus parameters in DT for CDF, but this hasn't
> >> been finalized yet.
> >>
> > 
> > While I understand this, I don't see why CDF can't enhance these
> > bindings if it has
> > requirements > than they have without disturbing the panel ones,
> > 
> > is DT really that inflexible?
> > 
> > It seems that have a simple description for basic panels like Thierry wants
> > to support, that can be enhanced for the other cases in the future should
> > suffice, I really don't like blocking stuff that makes things work on the chance
> > of something that isn't upstream yet, its sets a bad precedent, its also breaks
> > the perfect is the enemy of good rule
> 
> Just my opinion, but yes, DT is that inflexible. DT bindings are like a
> syscall. Once they are there, you need to support them forever. That
> support can, of course, include some kind of DT data converters or such,
> that try to convert old bindings to new bindings at kernel boot.

That's not entirely true. DT bindings are allowed to change, the only
restriction being that they don't break backwards compatibility. So if
there's ever a need to support new functionality, be it in the form of
new nodes or properties to support something like CDF, that's not a
problem as long as the code continues to work with existing bindings.

> In practice even such converter may be enough, if some important piece
> of data in the new bindings is missing, and this data was implicit in a
> driver. In that case the conversion, or parts of it, must be done later,
> in that specific driver.
> 
> Even that may be difficult, if the piece of data should actually be
> handled by some other driver. In that case there's probably need for
> some kind of global look-up table or such, so that the drivers can work
> around the problem of missing information.
> 
> I've been working with DT bindings for OMAP display subsystem for
> something like 1.5 years. Still not merged, as they are not perfect, and
> I'm scared to push them in non-perfect form, as that could result in
> some kind of DT-binding-converter-hell described above.

Am I the only one refusing to accept that we can't provide even the most
basic functionality simply because we haven't been able to come up with
the ultimately "perfect" DT binding yet? By definition it's not very
likely that any binding will ever be perfect.

I don't think we're doing ourselves any good by letting DT actively
hinder us in merging any of these features. I would personally prefer
having to support several bindings rather than not be able to provide
the functionality at all.

For crying out loud, we can use the 3D engine to render to a framebuffer
but we can't look at the result because we can't make the bloody panel
light up! Seriously!?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH] tmio: call tmiofb_set_par in tmiofb_probe
From: Dmitry Eremin-Solenikov @ 2013-10-17  9:50 UTC (permalink / raw)
  To: linux-fbdev

Call tmiofb_set_par() to initlaize fixed variable before registering
framebuffer. This is necessary for current kexecboot.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/video/tmiofb.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/video/tmiofb.c b/drivers/video/tmiofb.c
index deb8733..69fd0f7 100644
--- a/drivers/video/tmiofb.c
+++ b/drivers/video/tmiofb.c
@@ -774,6 +774,10 @@ static int tmiofb_probe(struct platform_device *dev)
 	if (retval)
 		goto err_hw_init;
 
+	retval = tmiofb_set_par(info);
+	if (retval)
+		goto err_set_par;
+
 	fb_videomode_to_modelist(data->modes, data->num_modes,
 				 &info->modelist);
 
@@ -787,7 +791,7 @@ static int tmiofb_probe(struct platform_device *dev)
 	return 0;
 
 err_register_framebuffer:
-/*err_set_par:*/
+err_set_par:
 	tmiofb_hw_stop(dev);
 err_hw_init:
 	if (cell->disable)
-- 
1.8.4.rc3


^ permalink raw reply related

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Tomi Valkeinen @ 2013-10-17  9:52 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dave Airlie, Laurent Pinchart, Laurent Pinchart, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <20131017091614.GC2502-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>

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

On 17/10/13 12:16, Thierry Reding wrote:
> On Thu, Oct 17, 2013 at 11:49:52AM +0300, Tomi Valkeinen wrote:

>> Just my opinion, but yes, DT is that inflexible. DT bindings are like a
>> syscall. Once they are there, you need to support them forever. That
>> support can, of course, include some kind of DT data converters or such,
>> that try to convert old bindings to new bindings at kernel boot.
> 
> That's not entirely true. DT bindings are allowed to change, the only
> restriction being that they don't break backwards compatibility. So if

True, but afaik the same goes for syscalls. But yes, it's probably
easier to add, say, new properties to DT bindings than adding new flags
to a syscall.

> Am I the only one refusing to accept that we can't provide even the most
> basic functionality simply because we haven't been able to come up with
> the ultimately "perfect" DT binding yet? By definition it's not very
> likely that any binding will ever be perfect.

With "perfect" I meant without any obvious features missing. I agree
that DT bindings will never be perfect, but I at least would like them
to be good enough to cover all the cases we know of.

> I don't think we're doing ourselves any good by letting DT actively
> hinder us in merging any of these features. I would personally prefer
> having to support several bindings rather than not be able to provide
> the functionality at all.

I'd say the driver writer is of course free to create basic bindings for
the device in question as long as the bindings are sane, and he agrees
to later create any needed converters. I don't see any problem with
having a driver supporting several versions of the bindings.

But I don't think it's fair to push a driver with basic bindings,
knowing that the bindings won't be enough in the future, and leave all
the converter-mess to other people (not saying that's the case here).

As a fbdev maintainer, I'm ok with adding DT bindings to device specific
fb drivers, as those can be just left as they are. Even when CDF is
merged, the specific fb drivers just work with the old bindings. If the
driver maintainer wants to use CDF, it's up to him to create any needed
binding-converters.

But a generic driver for panels is more problematic, as that one should
be maintained and refreshed to new features like CDF. If such was
proposed to be merged to fbdev, I'd be very reluctant to merge it if
there were any concerns about it.

I guess one option there would be to implement a new driver for the same
panel devices, one that supports CDF, and leave the old one as is. Not
so nice option, though.

> For crying out loud, we can use the 3D engine to render to a framebuffer
> but we can't look at the result because we can't make the bloody panel
> light up! Seriously!?

Ah, but do you have DT bindings for the 3D engine yet, which represent
how different internal blocks in the 3D engine are connected? If not,
better start designing it! Then the problem is solved, you won't even
have working 3D engine. ;)

But seriously speaking, I'm with you. I don't get this DT stuff.

And the funny thing is, DT should just represent the hardware, it
doesn't matter what kind of SW drivers there are, so in a sense talking
about DT problems with a SW framework like CDF is a bit silly. How hard
can it be to describe the hardware properly?

Very, obviously =).

 Tomi



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

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Tomi Valkeinen @ 2013-10-17 10:22 UTC (permalink / raw)
  To: Thierry Reding, Laurent Pinchart
  Cc: Laurent Pinchart, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <20131017085342.GB2502-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>

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

On 17/10/13 11:53, Thierry Reding wrote:

> I keep wondering why we absolutely must have compatibility between CDF
> and this simple panel binding. DT content is supposed to concern itself
> with the description of hardware only. What about the following isn't an
> accurate description of the panel hardware?
> 
> 	panel: panel {
> 		compatible = "cptt,claa101wb01";
> 
> 		power-supply = <&vdd_pnl_reg>;
> 		enable-gpios = <&gpio 90 0>;
> 
> 		backlight = <&backlight>;
> 	};
> 
> 	dsi-controller {
> 		panel = <&panel>;
> 	};

That's quite similar to how my current out-of-mainline OMAP DSS DT
bindings work. The difference is that I have a reference from the panel
node to the video source (dsi-controller), instead of the other way
around. I just find it more natural. It works the same way as, say,
regulators, gpios, etc.

However, one thing unclear is where the panel node should be. You seem
to have a DSI panel here. If the panel is truly not controlled in any
way, maybe having the panel as a normal platform device is ok. But if
the DSI panel is controlled with DSI messages, should it be a child of
the dsi-controller node, the same way i2c peripherals are children of
i2c master?

>>> +static const struct drm_display_mode auo_b101aw03_mode = {
>>> +	.clock = 51450,
>>> +	.hdisplay = 1024,
>>> +	.hsync_start = 1024 + 156,
>>> +	.hsync_end = 1024 + 156 + 8,
>>> +	.htotal = 1024 + 156 + 8 + 156,
>>> +	.vdisplay = 600,
>>> +	.vsync_start = 600 + 16,
>>> +	.vsync_end = 600 + 16 + 6,
>>> +	.vtotal = 600 + 16 + 6 + 16,
>>> +	.vrefresh = 60,
>>> +};
>>
>> Instead of hardcoding the modes in the driver, which would then require to be 
>> updated for every new simple panel model (and we know there are lots of them), 
>> why don't you specify the modes in the panel DT node ? The simple panel driver 
>> would then become much more generic. It would also allow board designers to 
>> tweak h/v sync timings depending on the system requirements.
> 
> Sigh... we keep second-guessing ourselves. Back at the time when power
> sequences were designed (and NAKed at the last minute), we all decided
> that the right thing to do would be to use specific compatible values
> for individual panels, because that would allow us to encode the power
> sequencing within the driver. And when we already have the panel model
> encoded in the compatible value, we might just as well encode the mode
> within the driver for that panel. Otherwise we'll have to keep adding
> the same mode timings for every board that uses the same panel.

Oh, I didn't feel "we all decided that the right thing..." =).

> I do agree though that it might be useful to tweak the mode in case the
> default one doesn't work. How about we provide a means to override the
> mode encoded in the driver using one specified in the DT? That's
> obviously a backwards-compatible change, so it could be added if or when
> it becomes necessary.

This sounds good to me.

Although maybe it'd be good to have the driver compatible with something
like "generic-panel", so that you could have:

compatible = "foo,specific-panel", "generic-panel";

and if there's no need for any power/gpio setup for your board, you may
skip adding "foo,specific-panel" support to the panel driver. Later,
somebody else may need to implement fine grained power/gpio support for
"foo,specific-panel", and it would just work.

Maybe that would help us with the problem of adding support in the
driver for a hundred different simple panels each used only once on a
specific board.

 Tomi



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

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-17 10:48 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Dave Airlie, Laurent Pinchart, Laurent Pinchart, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <525FB368.6020003-l0cyMroinI0@public.gmane.org>

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

On Thu, Oct 17, 2013 at 12:52:40PM +0300, Tomi Valkeinen wrote:
> On 17/10/13 12:16, Thierry Reding wrote:
> > On Thu, Oct 17, 2013 at 11:49:52AM +0300, Tomi Valkeinen wrote:
> 
> >> Just my opinion, but yes, DT is that inflexible. DT bindings are like a
> >> syscall. Once they are there, you need to support them forever. That
> >> support can, of course, include some kind of DT data converters or such,
> >> that try to convert old bindings to new bindings at kernel boot.
> > 
> > That's not entirely true. DT bindings are allowed to change, the only
> > restriction being that they don't break backwards compatibility. So if
> 
> True, but afaik the same goes for syscalls. But yes, it's probably
> easier to add, say, new properties to DT bindings than adding new flags
> to a syscall.
> 
> > Am I the only one refusing to accept that we can't provide even the most
> > basic functionality simply because we haven't been able to come up with
> > the ultimately "perfect" DT binding yet? By definition it's not very
> > likely that any binding will ever be perfect.
> 
> With "perfect" I meant without any obvious features missing. I agree
> that DT bindings will never be perfect, but I at least would like them
> to be good enough to cover all the cases we know of.

As far as the simple panel device tree binding is concerned, it covers
all the cases I know of. I can't seriously be expected to do any better
than that. These panels are dumb. They only come with a power supply
that needs to be switched on and an additional GPIO to enable it once
powered up. All three panels just work with that set of properties. So
anything that doesn't can arguably be covered by some other, not-so-
simple binding.

> > I don't think we're doing ourselves any good by letting DT actively
> > hinder us in merging any of these features. I would personally prefer
> > having to support several bindings rather than not be able to provide
> > the functionality at all.
> 
> I'd say the driver writer is of course free to create basic bindings for
> the device in question as long as the bindings are sane, and he agrees
> to later create any needed converters. I don't see any problem with
> having a driver supporting several versions of the bindings.
> 
> But I don't think it's fair to push a driver with basic bindings,
> knowing that the bindings won't be enough in the future, and leave all
> the converter-mess to other people (not saying that's the case here).
> 
> As a fbdev maintainer, I'm ok with adding DT bindings to device specific
> fb drivers, as those can be just left as they are. Even when CDF is
> merged, the specific fb drivers just work with the old bindings. If the
> driver maintainer wants to use CDF, it's up to him to create any needed
> binding-converters.
> 
> But a generic driver for panels is more problematic, as that one should
> be maintained and refreshed to new features like CDF. If such was
> proposed to be merged to fbdev, I'd be very reluctant to merge it if
> there were any concerns about it.

I'm still missing the point here. For one, I don't see any reason that
this driver would ever need to gain CDF support. At the same time, CDF
could easily be supported by just adding a few required properties and
it should be easy to support any non-CDF cases just as well to remain
backwards-compatible.

> I guess one option there would be to implement a new driver for the same
> panel devices, one that supports CDF, and leave the old one as is. Not
> so nice option, though.

As I said, anything that really needs a CDF binding to work likely isn't
"simple" anymore, therefore a separate driver can easily be justified.

> > For crying out loud, we can use the 3D engine to render to a framebuffer
> > but we can't look at the result because we can't make the bloody panel
> > light up! Seriously!?
> 
> Ah, but do you have DT bindings for the 3D engine yet, which represent
> how different internal blocks in the 3D engine are connected? If not,
> better start designing it! Then the problem is solved, you won't even
> have working 3D engine. ;)
> 
> But seriously speaking, I'm with you. I don't get this DT stuff.
> 
> And the funny thing is, DT should just represent the hardware, it
> doesn't matter what kind of SW drivers there are, so in a sense talking
> about DT problems with a SW framework like CDF is a bit silly. How hard
> can it be to describe the hardware properly?
> 
> Very, obviously =).

Indeed. The fundamental point here seems to be that we don't represent
things in DT which we don't need. In the same way that we don't add
device nodes for enumerable devices, why would we need to explicitly add
information about connections between devices that cannot be controlled
anyway. If the board connects an RGB/LVDS output to a panel directly, it
is only required that the output knows what it is connected to because
there is no other way besides DT to obtain any information about the
panel. Therefore a unidirectional connection is more than enough. Using
that the output can access the panel and query it for information such
as the mode timings as well as switch it on or off as required.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Laurent Pinchart @ 2013-10-17 11:02 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Laurent Pinchart, Tomi Valkeinen, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <20131017085342.GB2502-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>

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

Hi Thierry,

On Thursday 17 October 2013 10:53:43 Thierry Reding wrote:
> Adding the dri-devel mailing list on Cc, since this discussion is
> outgrowing its original intent.
> 
> On Thu, Oct 17, 2013 at 02:47:52AM +0200, Laurent Pinchart wrote:
> > Hi Thierry,
> > 
> > Thank you for the patch.
> > 
> > On Wednesday 16 October 2013 20:25:12 Thierry Reding wrote:
> > > Add a driver for simple panels. Such panels can have a regulator that
> > > provides the supply voltage and a separate GPIO to enable the panel.
> > > Optionally the panels can have a backlight associated with them so it
> > > can be enabled or disabled according to the panel's power management
> > > mode.
> > > 
> > > Support is added for three panels: An AU Optronics 10.1" WSVGA, a
> > > Chunghwa Picture Tubes 10.1" WXGA and a Panasonic 10.1 WUXGA TFT LCD
> > > panel.
> > > 
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > > 
> > >  .../devicetree/bindings/panel/auo,b101aw03.txt     |   7 +
> > >  .../bindings/panel/chunghwa,claa101wb03.txt        |   7 +
> > >  .../bindings/panel/panasonic,vvx10f004b00.txt      |   7 +
> > >  .../devicetree/bindings/panel/simple-panel.txt     |  21 ++
> > >  drivers/gpu/drm/Makefile                           |   1 +
> > >  drivers/gpu/drm/panel/Kconfig                      |  13 +
> > >  drivers/gpu/drm/panel/Makefile                     |   1 +
> > >  drivers/gpu/drm/panel/panel-simple.c               | 335 ++++++++++++++
> > >  8 files changed, 392 insertions(+)
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/panel/simple-panel.txt
> > >  create mode 100644 drivers/gpu/drm/panel/Makefile
> > >  create mode 100644 drivers/gpu/drm/panel/panel-simple.c

[snip]

> > > diff --git a/Documentation/devicetree/bindings/panel/simple-panel.txt
> > > b/Documentation/devicetree/bindings/panel/simple-panel.txt new file mode
> > > 100644
> > > index 0000000..1341bbf
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/panel/simple-panel.txt
> > > @@ -0,0 +1,21 @@
> > > +Simple display panel
> > > +
> > > +Required properties:
> > > +- power-supply: regulator to provide the supply voltage
> > > +
> > > +Optional properties:
> > > +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> > > +- enable-gpios: GPIO pin to enable or disable the panel
> > > +- backlight: phandle of the backlight device attached to the panel
> > > +
> > > +Example:
> > > +
> > > +	panel: panel {
> > > +		compatible = "cptt,claa101wb01";
> > > +		ddc-i2c-bus = <&panelddc>;
> > > +
> > > +		power-supply = <&vdd_pnl_reg>;
> > > +		enable-gpios = <&gpio 90 0>;
> > > +
> > > +		backlight = <&backlight>;
> > > +	};
> > 
> > My biggest concern here is that this will not be compatible with the CDF
> > DT bindings. They're not complete yet, but they will require connections
> > between entities to be described in DT, in a way very similar (or
> > actually identical) to the V4L2 DT bindings, documented in
> > Documentation/devicetree/bindings/media/video-interfaces.txt. Could you
> > have a look at that ? Please ignore all optional properties beside
> > remote-endpoint, as they're V4L2 specific.
> 
> Okay, so if I understand correctly, translating those bindings to panel
> nodes would look somewhat like this:
> 
> 	dc: display-controller {
> 		ports {
> 			port@0 {
> 				remote-endpoint = <&panel>;
> 			};
> 		};
> 	};
> 
> 	panel: panel {
> 		ports {
> 			port@0 {
> 				remote-endpoint = <&dc>;
> 			};
> 		};
> 	};
> 
> The above leaves out any of the other, non-relevant properties. Does
> that sound about right?

Yes it does.

> That seems like an overly complex amount of data to describe just a simple
> panel where the only connection between it and the display hardware is the
> data bus and I was sort of expecting the CDF to provide some sort of
> shortcut for setups where the connection is 1:1 with no means to perform any
> configuration of the bus.

CDF needs to model video pipelines that can be more complex than just a 
display controller with a single output connected to a panel with a single 
input. For that reason the DT bindings need to describe connections in a 
generic enough way. The above DT fragment might be slightly more complex than 
one would expect when thinking about the really simple case only, but it's 
nowhere near overly complex in my opinion.

Please note that, when a device has as single port, the ports node can be 
omitted, and the port doesn't need to be numbered. You would then end up with

 	dc: display-controller {
		port {
			remote-endpoint = <&panel>;
 		};
 	};
 
 	panel: panel {
 		port {
 			remote-endpoint = <&dc>;
 		};
 	};

I don't think there's a way to simplify it further.

> > I also plan to specify video bus parameters in DT for CDF, but this hasn't
> > been finalized yet.
> 
> That effectively blocks any of the code that I've been working on until
> CDF has been merged. It's been discussed for over a year now, and there
> has even been significant pushback on using CDF in DRM, so even if CDF
> is eventually merged, that will still leave DRM with no way to turn on
> a simple panel.

I'm probably even more concerned about the long time all this took and will 
still take than you are, sincerely. I'm obviously to blame for not being able 
to explain the problems and solutions correctly, as I see no way why people 
would push back if they realized that the other solutions that have been 
proposed, far from being future-proof, won't even be present-proof. We would 
be shooting ourselves in the foot, but what can I do when too many people want 
to do so ? I'll still keep trying of course, but not for years.I have yet to 
see someone proposing a viable solution to share drivers between DRM and V4L2, 
which is a real pressing requirement, partly due to DT.

Regarding the use of CDF in DRM, please note that it will be optional for 
drivers to switch to the new framework. Nothing should hopefully require 
existing drivers to be converted, drivers that want to make use of CDF to 
support panels will be welcome to do so, but they big DRM drivers for desktop 
GPUs won't be affected.

> I keep wondering why we absolutely must have compatibility between CDF
> and this simple panel binding. DT content is supposed to concern itself
> with the description of hardware only. What about the following isn't an
> accurate description of the panel hardware?
> 
> 	panel: panel {
> 		compatible = "cptt,claa101wb01";
> 
> 		power-supply = <&vdd_pnl_reg>;
> 		enable-gpios = <&gpio 90 0>;
> 
> 		backlight = <&backlight>;
> 	};
> 
> 	dsi-controller {
> 		panel = <&panel>;
> 	};
> 
> Note how the above is also not incompatible with anything that the CDF
> (to be) mandates, so it could easily be extended with any nodes or
> properties that CDF needs to work properly without breaking backwards-
> compatibility.

As mentioned above, CDF needs to describe more complex pipelines. To make the 
driver life as simple as possible the core code will handle the pipeline 
description in the DT bindings, and it does require it to be generic enough. 
You can of course describe connections in various ways depending on the 
hardware, which would make the DT-related code needlessly overcomplex. One of 
the issue with the above description is that there's no way to go from the 
panel to the display controller, which might be a show stopper in the future 
when code requires that. A workaround would be to look through the whole DT 
for the reverse link, which would be pretty inefficient.

> > > diff --git a/drivers/gpu/drm/panel/panel-simple.c

[...]

> > > +static void panel_simple_enable(struct drm_panel *panel)
> > > +{
> > > +	struct panel_simple *p = to_panel_simple(panel);
> > > +	int err;
> > > +
> > > +	if (p->enabled)
> > > +		return;
> > > +
> > > +	err = regulator_enable(p->supply);
> > > +	if (err < 0)
> > > +		dev_err(panel->dev, "failed to enable supply: %d\n", err);
> > 
> > Is that really a non-fatal error ? Shouldn't the enable operation return
> > an int ?
> 
> There's no way to propagate this in DRM, so why go through the trouble
> of returning the error? Furthermore, there's nothing that the caller
> could do to remedy the situation anyway.

That's a DRM issue, which could be fixed. While the caller can't remedy the 
situation, it should at least report the error to the application instead of 
silently ignoring it.

> > > +static const struct drm_display_mode auo_b101aw03_mode = {
> > > +	.clock = 51450,
> > > +	.hdisplay = 1024,
> > > +	.hsync_start = 1024 + 156,
> > > +	.hsync_end = 1024 + 156 + 8,
> > > +	.htotal = 1024 + 156 + 8 + 156,
> > > +	.vdisplay = 600,
> > > +	.vsync_start = 600 + 16,
> > > +	.vsync_end = 600 + 16 + 6,
> > > +	.vtotal = 600 + 16 + 6 + 16,
> > > +	.vrefresh = 60,
> > > +};
> > 
> > Instead of hardcoding the modes in the driver, which would then require to
> > be updated for every new simple panel model (and we know there are lots
> > of them), why don't you specify the modes in the panel DT node ? The
> > simple panel driver would then become much more generic. It would also
> > allow board designers to tweak h/v sync timings depending on the system
> > requirements.
> 
> Sigh... we keep second-guessing ourselves. Back at the time when power
> sequences were designed (and NAKed at the last minute), we all decided
> that the right thing to do would be to use specific compatible values
> for individual panels, because that would allow us to encode the power
> sequencing within the driver. And when we already have the panel model
> encoded in the compatible value, we might just as well encode the mode
> within the driver for that panel. Otherwise we'll have to keep adding
> the same mode timings for every board that uses the same panel.
> 
> I do agree though that it might be useful to tweak the mode in case the
> default one doesn't work. How about we provide a means to override the
> mode encoded in the driver using one specified in the DT? That's
> obviously a backwards-compatible change, so it could be added if or when
> it becomes necessary.

I share Tomi's point of view here. Your three panels use the same power 
sequence, so I believe we should have a generic panel compatible string that 
would use modes described in DT for the common case. Only if a panel required 
something more complex which can't (or which could, but won't for any reason) 
accurately be described in DT would you need to extend the driver.

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-17 11:05 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Laurent Pinchart, Laurent Pinchart, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <525FBA5D.9000001-l0cyMroinI0@public.gmane.org>

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

On Thu, Oct 17, 2013 at 01:22:21PM +0300, Tomi Valkeinen wrote:
> On 17/10/13 11:53, Thierry Reding wrote:
> 
> > I keep wondering why we absolutely must have compatibility between CDF
> > and this simple panel binding. DT content is supposed to concern itself
> > with the description of hardware only. What about the following isn't an
> > accurate description of the panel hardware?
> > 
> > 	panel: panel {
> > 		compatible = "cptt,claa101wb01";
> > 
> > 		power-supply = <&vdd_pnl_reg>;
> > 		enable-gpios = <&gpio 90 0>;
> > 
> > 		backlight = <&backlight>;
> > 	};
> > 
> > 	dsi-controller {
> > 		panel = <&panel>;
> > 	};
> 
> That's quite similar to how my current out-of-mainline OMAP DSS DT
> bindings work. The difference is that I have a reference from the panel
> node to the video source (dsi-controller), instead of the other way
> around. I just find it more natural. It works the same way as, say,
> regulators, gpios, etc.

I suppose that depends on the way you look at it. In the above proposal
I consider the output (DSI controller) to use the panel as a resource
that provides a certain set of services (query mode timings, provide a
way to enable or disable the panel). Similarly the panel uses the
backlight as a resource that provides a certain set of services (such as
changing the brightness).

The above also follows the natural order of control. The panel has no
way to control the DSI output. However, it is the output that controls
when a panel is required and turn it on as appropriate.

> However, one thing unclear is where the panel node should be. You seem
> to have a DSI panel here. If the panel is truly not controlled in any
> way, maybe having the panel as a normal platform device is ok. But if
> the DSI panel is controlled with DSI messages, should it be a child of
> the dsi-controller node, the same way i2c peripherals are children of
> i2c master?

That's one possibility. In this particular case it's not even necessary
to use any of DSIs control methods to talk to the panel. The panel only
receives the data stream and "just works".

Even if the panel were to be controlled via DSI, I think keeping it as
a separate device node is equally valid. It's really boils down to what
I already said above. The output uses the panel as a resource, similar
to how other devices might use a GPIO controller to use a GPIO pin, or
an interrupt controller to request an IRQ.

> >>> +static const struct drm_display_mode auo_b101aw03_mode = {
> >>> +	.clock = 51450,
> >>> +	.hdisplay = 1024,
> >>> +	.hsync_start = 1024 + 156,
> >>> +	.hsync_end = 1024 + 156 + 8,
> >>> +	.htotal = 1024 + 156 + 8 + 156,
> >>> +	.vdisplay = 600,
> >>> +	.vsync_start = 600 + 16,
> >>> +	.vsync_end = 600 + 16 + 6,
> >>> +	.vtotal = 600 + 16 + 6 + 16,
> >>> +	.vrefresh = 60,
> >>> +};
> >>
> >> Instead of hardcoding the modes in the driver, which would then require to be 
> >> updated for every new simple panel model (and we know there are lots of them), 
> >> why don't you specify the modes in the panel DT node ? The simple panel driver 
> >> would then become much more generic. It would also allow board designers to 
> >> tweak h/v sync timings depending on the system requirements.
> > 
> > Sigh... we keep second-guessing ourselves. Back at the time when power
> > sequences were designed (and NAKed at the last minute), we all decided
> > that the right thing to do would be to use specific compatible values
> > for individual panels, because that would allow us to encode the power
> > sequencing within the driver. And when we already have the panel model
> > encoded in the compatible value, we might just as well encode the mode
> > within the driver for that panel. Otherwise we'll have to keep adding
> > the same mode timings for every board that uses the same panel.
> 
> Oh, I didn't feel "we all decided that the right thing..." =).
> 
> > I do agree though that it might be useful to tweak the mode in case the
> > default one doesn't work. How about we provide a means to override the
> > mode encoded in the driver using one specified in the DT? That's
> > obviously a backwards-compatible change, so it could be added if or when
> > it becomes necessary.
> 
> This sounds good to me.
> 
> Although maybe it'd be good to have the driver compatible with something
> like "generic-panel", so that you could have:
> 
> compatible = "foo,specific-panel", "generic-panel";
> 
> and if there's no need for any power/gpio setup for your board, you may
> skip adding "foo,specific-panel" support to the panel driver. Later,
> somebody else may need to implement fine grained power/gpio support for
> "foo,specific-panel", and it would just work.
> 
> Maybe that would help us with the problem of adding support in the
> driver for a hundred different simple panels each used only once on a
> specific board.

Sure, that all sounds like reasonable additions. All of the suggestions
are even backwards-compatible and hence can be added when needed without
breaking compatibility with existing users of the binding.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Laurent Pinchart @ 2013-10-17 11:07 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Tomi Valkeinen, Dave Airlie, Laurent Pinchart, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <20131017104856.GA10993-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>

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

Hi Thierry,

On Thursday 17 October 2013 12:48:57 Thierry Reding wrote:
> On Thu, Oct 17, 2013 at 12:52:40PM +0300, Tomi Valkeinen wrote:
> > On 17/10/13 12:16, Thierry Reding wrote:
> > > On Thu, Oct 17, 2013 at 11:49:52AM +0300, Tomi Valkeinen wrote:
> > >> Just my opinion, but yes, DT is that inflexible. DT bindings are like a
> > >> syscall. Once they are there, you need to support them forever. That
> > >> support can, of course, include some kind of DT data converters or
> > >> such, that try to convert old bindings to new bindings at kernel boot.
> > > 
> > > That's not entirely true. DT bindings are allowed to change, the only
> > > restriction being that they don't break backwards compatibility. So if
> > 
> > True, but afaik the same goes for syscalls. But yes, it's probably
> > easier to add, say, new properties to DT bindings than adding new flags
> > to a syscall.
> > 
> > > Am I the only one refusing to accept that we can't provide even the most
> > > basic functionality simply because we haven't been able to come up with
> > > the ultimately "perfect" DT binding yet? By definition it's not very
> > > likely that any binding will ever be perfect.
> > 
> > With "perfect" I meant without any obvious features missing. I agree
> > that DT bindings will never be perfect, but I at least would like them
> > to be good enough to cover all the cases we know of.
> 
> As far as the simple panel device tree binding is concerned, it covers
> all the cases I know of. I can't seriously be expected to do any better
> than that. These panels are dumb. They only come with a power supply
> that needs to be switched on and an additional GPIO to enable it once
> powered up. All three panels just work with that set of properties. So
> anything that doesn't can arguably be covered by some other, not-so-
> simple binding.
> 
> > > I don't think we're doing ourselves any good by letting DT actively
> > > hinder us in merging any of these features. I would personally prefer
> > > having to support several bindings rather than not be able to provide
> > > the functionality at all.
> > 
> > I'd say the driver writer is of course free to create basic bindings for
> > the device in question as long as the bindings are sane, and he agrees
> > to later create any needed converters. I don't see any problem with
> > having a driver supporting several versions of the bindings.
> > 
> > But I don't think it's fair to push a driver with basic bindings,
> > knowing that the bindings won't be enough in the future, and leave all
> > the converter-mess to other people (not saying that's the case here).
> > 
> > As a fbdev maintainer, I'm ok with adding DT bindings to device specific
> > fb drivers, as those can be just left as they are. Even when CDF is
> > merged, the specific fb drivers just work with the old bindings. If the
> > driver maintainer wants to use CDF, it's up to him to create any needed
> > binding-converters.
> > 
> > But a generic driver for panels is more problematic, as that one should
> > be maintained and refreshed to new features like CDF. If such was
> > proposed to be merged to fbdev, I'd be very reluctant to merge it if
> > there were any concerns about it.
> 
> I'm still missing the point here. For one, I don't see any reason that
> this driver would ever need to gain CDF support. At the same time, CDF
> could easily be supported by just adding a few required properties and
> it should be easy to support any non-CDF cases just as well to remain
> backwards-compatible.
> 
> > I guess one option there would be to implement a new driver for the same
> > panel devices, one that supports CDF, and leave the old one as is. Not
> > so nice option, though.
> 
> As I said, anything that really needs a CDF binding to work likely isn't
> "simple" anymore, therefore a separate driver can easily be justified.

The system as a whole would be more complex, but the panel could be the same. 
We can't have two drivers for the same piece of hardware in the DT world, as 
there will be a single compatible string and no way to choose between the 
drivers (unlike the board code world that could set device names to "foo-
encoder-v4l2" or "foo-encoder-drm" and live happily with that ever after).

> > > For crying out loud, we can use the 3D engine to render to a framebuffer
> > > but we can't look at the result because we can't make the bloody panel
> > > light up! Seriously!?
> > 
> > Ah, but do you have DT bindings for the 3D engine yet, which represent
> > how different internal blocks in the 3D engine are connected? If not,
> > better start designing it! Then the problem is solved, you won't even
> > have working 3D engine. ;)
> > 
> > But seriously speaking, I'm with you. I don't get this DT stuff.
> > 
> > And the funny thing is, DT should just represent the hardware, it
> > doesn't matter what kind of SW drivers there are, so in a sense talking
> > about DT problems with a SW framework like CDF is a bit silly. How hard
> > can it be to describe the hardware properly?
> > 
> > Very, obviously =).
> 
> Indeed. The fundamental point here seems to be that we don't represent
> things in DT which we don't need. In the same way that we don't add device
> nodes for enumerable devices, why would we need to explicitly add
> information about connections between devices that cannot be controlled
> anyway. If the board connects an RGB/LVDS output to a panel directly, it is
> only required that the output knows what it is connected to because there is
> no other way besides DT to obtain any information about the panel. Therefore
> a unidirectional connection is more than enough. Using that the output can
> access the panel and query it for information such as the mode timings as
> well as switch it on or off as required.

Please note that the idea of putting the modes in DT in no way implies that 
the device controller should parse the modes from the panel node. It must 
instead query the panel at runtime through whatever API is provided. Going 
directly to the DT data would be a layering violation. The reason why I 
propose to add modes to the panel DT node is to make the geenric panel driver 
more generic and simpler.

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Laurent Pinchart @ 2013-10-17 11:15 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Tomi Valkeinen, Laurent Pinchart, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <20131017110517.GC10993-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>

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

Hi Thierry,

On Thursday 17 October 2013 13:05:18 Thierry Reding wrote:
> On Thu, Oct 17, 2013 at 01:22:21PM +0300, Tomi Valkeinen wrote:
> > On 17/10/13 11:53, Thierry Reding wrote:
> > > I keep wondering why we absolutely must have compatibility between CDF
> > > and this simple panel binding. DT content is supposed to concern itself
> > > with the description of hardware only. What about the following isn't an
> > > accurate description of the panel hardware?
> > > 
> > > 	panel: panel {
> > > 		compatible = "cptt,claa101wb01";
> > > 		
> > > 		power-supply = <&vdd_pnl_reg>;
> > > 		enable-gpios = <&gpio 90 0>;
> > > 		
> > > 		backlight = <&backlight>;
> > > 	};
> > > 	
> > > 	dsi-controller {
> > > 		panel = <&panel>;
> > > 	};
> > 
> > That's quite similar to how my current out-of-mainline OMAP DSS DT
> > bindings work. The difference is that I have a reference from the panel
> > node to the video source (dsi-controller), instead of the other way
> > around. I just find it more natural. It works the same way as, say,
> > regulators, gpios, etc.
> 
> I suppose that depends on the way you look at it. In the above proposal
> I consider the output (DSI controller) to use the panel as a resource
> that provides a certain set of services (query mode timings, provide a
> way to enable or disable the panel). Similarly the panel uses the
> backlight as a resource that provides a certain set of services (such as
> changing the brightness).
> 
> The above also follows the natural order of control. The panel has no
> way to control the DSI output. However, it is the output that controls
> when a panel is required and turn it on as appropriate.

I'm no DSI expert, but I know enough about it to be sure that Tomi will 
disagree. DSI panels can have complex power sequences that require the input 
stream to be finely controlled (for instance it might require a clock without 
video frames for some time, switch a GPIO or regulator, send a command to the 
panel, and then only get video frames). For that reason all developers I've 
talked to who have an in-depth knowledge of DSI and DSI panels told me that 
the panel needs to control the video bus, and request the video source to 
start/stop the video stream.

> > However, one thing unclear is where the panel node should be. You seem
> > to have a DSI panel here. If the panel is truly not controlled in any
> > way, maybe having the panel as a normal platform device is ok. But if
> > the DSI panel is controlled with DSI messages, should it be a child of
> > the dsi-controller node, the same way i2c peripherals are children of
> > i2c master?
> 
> That's one possibility. In this particular case it's not even necessary
> to use any of DSIs control methods to talk to the panel. The panel only
> receives the data stream and "just works".
> 
> Even if the panel were to be controlled via DSI, I think keeping it as
> a separate device node is equally valid. It's really boils down to what
> I already said above. The output uses the panel as a resource, similar
> to how other devices might use a GPIO controller to use a GPIO pin, or
> an interrupt controller to request an IRQ.

Except that the display controller can also provides resources to the panel. 
For DSI panels that support DSI commands, the control bus is provided by the 
display controller. Much like an I2C device must be a child of its I2C 
controller node, the DSI panel device should then be a child of the display 
controller node.

> > >>> +static const struct drm_display_mode auo_b101aw03_mode = {
> > >>> +	.clock = 51450,
> > >>> +	.hdisplay = 1024,
> > >>> +	.hsync_start = 1024 + 156,
> > >>> +	.hsync_end = 1024 + 156 + 8,
> > >>> +	.htotal = 1024 + 156 + 8 + 156,
> > >>> +	.vdisplay = 600,
> > >>> +	.vsync_start = 600 + 16,
> > >>> +	.vsync_end = 600 + 16 + 6,
> > >>> +	.vtotal = 600 + 16 + 6 + 16,
> > >>> +	.vrefresh = 60,
> > >>> +};
> > >> 
> > >> Instead of hardcoding the modes in the driver, which would then require
> > >> to be updated for every new simple panel model (and we know there are
> > >> lots of them), why don't you specify the modes in the panel DT node ?
> > >> The simple panel driver would then become much more generic. It would
> > >> also allow board designers to tweak h/v sync timings depending on the
> > >> system requirements.
> > > 
> > > Sigh... we keep second-guessing ourselves. Back at the time when power
> > > sequences were designed (and NAKed at the last minute), we all decided
> > > that the right thing to do would be to use specific compatible values
> > > for individual panels, because that would allow us to encode the power
> > > sequencing within the driver. And when we already have the panel model
> > > encoded in the compatible value, we might just as well encode the mode
> > > within the driver for that panel. Otherwise we'll have to keep adding
> > > the same mode timings for every board that uses the same panel.
> > 
> > Oh, I didn't feel "we all decided that the right thing..." =).
> > 
> > > I do agree though that it might be useful to tweak the mode in case the
> > > default one doesn't work. How about we provide a means to override the
> > > mode encoded in the driver using one specified in the DT? That's
> > > obviously a backwards-compatible change, so it could be added if or when
> > > it becomes necessary.
> > 
> > This sounds good to me.
> > 
> > Although maybe it'd be good to have the driver compatible with something
> > like "generic-panel", so that you could have:
> > 
> > compatible = "foo,specific-panel", "generic-panel";
> > 
> > and if there's no need for any power/gpio setup for your board, you may
> > skip adding "foo,specific-panel" support to the panel driver. Later,
> > somebody else may need to implement fine grained power/gpio support for
> > "foo,specific-panel", and it would just work.
> > 
> > Maybe that would help us with the problem of adding support in the
> > driver for a hundred different simple panels each used only once on a
> > specific board.
> 
> Sure, that all sounds like reasonable additions. All of the suggestions
> are even backwards-compatible and hence can be added when needed without
> breaking compatibility with existing users of the binding.

What's wrong with moving the three hardcoded modes we already have in the 
driver to DT before pushing this to mainline ?

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Tomi Valkeinen @ 2013-10-17 11:21 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dave Airlie, Laurent Pinchart, Laurent Pinchart, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <20131017104856.GA10993-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>

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

On 17/10/13 13:48, Thierry Reding wrote:

> As far as the simple panel device tree binding is concerned, it covers
> all the cases I know of. I can't seriously be expected to do any better
> than that. These panels are dumb. They only come with a power supply
> that needs to be switched on and an additional GPIO to enable it once
> powered up. All three panels just work with that set of properties. So
> anything that doesn't can arguably be covered by some other, not-so-
> simple binding.

Agreed. I thought the DT bindings were missing information about the
video path, but I didn't realize you have a reference from the video
output to the panel.

However, I think there's usually a bit more information needed than what
you describe above. All the dump panels I know have requirements on the
sequences related to the video signal, powers and gpios, and delays
between those.

I guess the most common enable scenario is something like:

- enable video signal
- enable power
- wait n msecs
- set GPIO

For some panels, the enable GPIO might actually be a reset GPIO, not
enable. Some could want the video signal only be enabled after the
enable/reset is done.

Anyway, I think what you describe fits most of the panels, so no need to
worry about the possible ones I described. I just wanted to point out
that there may be very dump panels that are still different.

> I'm still missing the point here. For one, I don't see any reason that
> this driver would ever need to gain CDF support. At the same time, CDF
> could easily be supported by just adding a few required properties and
> it should be easy to support any non-CDF cases just as well to remain
> backwards-compatible.

Well, I guess the point here is that if you have a SoC display
controller driver and panel drivers, and you want to support complex
panels using CDF, you need to implement CDF support into your SoC
display controller driver in addition to adding CDF support to the panel
driver.

And, you still want to be able to use the simple panels. So either you
support both CDF panels and non-CDF panels with your SoC dispc, which
could be a bit messy, or you add CDF support to all panel drivers you use.

>> I guess one option there would be to implement a new driver for the same
>> panel devices, one that supports CDF, and leave the old one as is. Not
>> so nice option, though.
> 
> As I said, anything that really needs a CDF binding to work likely isn't
> "simple" anymore, therefore a separate driver can easily be justified.

I think it's rarely the case that a panel specifically needs CDF. It's
more that CDF offers a common framework for display components, making
it easy to make them independent of the platform used. And if you make
your SoC support CDF, you are able to use all the CDF drivers already
implemented.

That said, I don't see anything technically preventing from having
support for both CDF and non-CDF panels. It's just more complex, and
they cannot be mixed freely. For example, if I have CDF support in my
SoC driver and a panel driver, I can easily add a CDF encoder driver in
between. But if the panel driver is a custom one, then I need a custom
encoder driver there also.

> Indeed. The fundamental point here seems to be that we don't represent
> things in DT which we don't need. In the same way that we don't add

Surely we should represent the things about the HW we don't need now, if
we know they might be needed in the future? (But that's not the case
here, see below)

> device nodes for enumerable devices, why would we need to explicitly add
> information about connections between devices that cannot be controlled
> anyway. If the board connects an RGB/LVDS output to a panel directly, it
> is only required that the output knows what it is connected to because
> there is no other way besides DT to obtain any information about the
> panel. Therefore a unidirectional connection is more than enough. Using
> that the output can access the panel and query it for information such
> as the mode timings as well as switch it on or off as required.

Right, in that case, you do have all the necessary information in the
DT. I don't think it really matters if the link in DT is from the SoC to
the panel, vice versa, or both ways. It still describes the same thing.
Even with unidirectional link you can always find the reverse link, you
just need to do more work going throught the DT data.

 Tomi



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

^ permalink raw reply

* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Tomi Valkeinen @ 2013-10-17 11:35 UTC (permalink / raw)
  To: Laurent Pinchart, Thierry Reding
  Cc: Laurent Pinchart, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <12566222.aRYc4MDoOm@avalon>

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

On 17/10/13 14:02, Laurent Pinchart wrote:

>> Okay, so if I understand correctly, translating those bindings to panel
>> nodes would look somewhat like this:
>>
>> 	dc: display-controller {
>> 		ports {
>> 			port@0 {
>> 				remote-endpoint = <&panel>;
>> 			};
>> 		};
>> 	};
>>
>> 	panel: panel {
>> 		ports {
>> 			port@0 {
>> 				remote-endpoint = <&dc>;
>> 			};
>> 		};
>> 	};
>>
>> The above leaves out any of the other, non-relevant properties. Does
>> that sound about right?
> 
> Yes it does.

It does?

Shouldn't it be something like:

panel {
	ports {
		port@0 {
			endpoint@0 {
				remote = <&dc>;
			};
		};
	};
};

And simplified:

panel {
	port {
		endpoint@0 {
			remote = <&dc>;
		};
	};
};

You do need a node for the endpoint, a remote-endpoint property is not
enough.

> Please note that, when a device has as single port, the ports node can be 
> omitted, and the port doesn't need to be numbered. You would then end up with
> 
>  	dc: display-controller {
> 		port {
> 			remote-endpoint = <&panel>;
>  		};
>  	};
>  
>  	panel: panel {
>  		port {
>  			remote-endpoint = <&dc>;
>  		};
>  	};
> 
> I don't think there's a way to simplify it further.

I'm not sure if there's a specific need for the port or endpoint nodes
in cases like the above. Even if we have common properties describing
the endpoint, I guess they could just be in the parent node.

panel {
	remote = <&dc>;
	common-video-property = <asd>;
};

The above would imply one port and one endpoint. Would that work? If we
had a function like parse_endpoint(node), we could just point it to
either a real endpoint node, or to the device's node.

 Tomi



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

^ permalink raw reply

* [PATCH 0/6] omapdss: hdmi: Add support for hdmi on OMAP5 and DRA7x SoCs
From: Archit Taneja @ 2013-10-17 11:38 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja

The HDMI IP on OMAP5/DRA7xx has very similar wrapper, PLL and PHY sub blocks
found in OMAP4 DSS. However, the HDMI core IP(which implements the HDMI
protocol) is completely new.

Create a library for configuring the new HDMI core IP, make minor modifications
in PLL and PHY. And finally, add a hdmi5 driver. This driver can eventually be
merged with the hdmi4 driver such that both of them use the same driver, we
would need to create core IP ops for that, we've left that for later.

Archit Taneja (6):
  omapdss: hdmi: support larger register offsets for OMAP5 HDMI core
  omapdss: hdmi: Add initial OMAP5/DRA7x HDMI core IP library
  omapdss: hdmi phy: Incorporate changes for OMAP5/DRA7x
  omapdss: hdmi pll: Incorporate changes for OMAP5/DRA7x
  omapdss: hdmi: add OMAP5/DRA7x hdmi driver
  omapdss: hdmi4/hdmi5: set regulator voltage to required level

 drivers/video/omap2/dss/Kconfig      |  10 +
 drivers/video/omap2/dss/Makefile     |   2 +
 drivers/video/omap2/dss/core.c       |   6 +
 drivers/video/omap2/dss/dss.h        |   3 +
 drivers/video/omap2/dss/hdmi.h       |   7 +-
 drivers/video/omap2/dss/hdmi4.c      |   5 +
 drivers/video/omap2/dss/hdmi5.c      | 704 ++++++++++++++++++++++++++++++
 drivers/video/omap2/dss/hdmi5_core.c | 812 +++++++++++++++++++++++++++++++++++
 drivers/video/omap2/dss/hdmi5_core.h | 298 +++++++++++++
 drivers/video/omap2/dss/hdmi_phy.c   |  94 +++-
 drivers/video/omap2/dss/hdmi_pll.c   |  74 +++-
 drivers/video/omap2/dss/hdmi_wp.c    |   2 +-
 12 files changed, 2008 insertions(+), 9 deletions(-)
 create mode 100644 drivers/video/omap2/dss/hdmi5.c
 create mode 100644 drivers/video/omap2/dss/hdmi5_core.c
 create mode 100644 drivers/video/omap2/dss/hdmi5_core.h

-- 
1.8.1.2


^ permalink raw reply

* [PATCH 1/6] omapdss: hdmi: support larger register offsets for OMAP5 HDMI core
From: Archit Taneja @ 2013-10-17 11:38 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1382009202-18984-1-git-send-email-archit@ti.com>

The HDMI core IP on OMAP5 has a wider register address range. The offsets for
these registers can't fit into the u16 type currently used for hdmi register
read and write functions. Use u32 for offsets instead.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/hdmi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.h b/drivers/video/omap2/dss/hdmi.h
index b049376..b410b14 100644
--- a/drivers/video/omap2/dss/hdmi.h
+++ b/drivers/video/omap2/dss/hdmi.h
@@ -360,13 +360,13 @@ struct hdmi_core_data {
 	struct hdmi_core_infoframe_avi avi_cfg;
 };
 
-static inline void hdmi_write_reg(void __iomem *base_addr, const u16 idx,
+static inline void hdmi_write_reg(void __iomem *base_addr, const u32 idx,
 		u32 val)
 {
 	__raw_writel(val, base_addr + idx);
 }
 
-static inline u32 hdmi_read_reg(void __iomem *base_addr, const u16 idx)
+static inline u32 hdmi_read_reg(void __iomem *base_addr, const u32 idx)
 {
 	return __raw_readl(base_addr + idx);
 }
-- 
1.8.1.2


^ permalink raw reply related

* [PATCH 2/6] omapdss: hdmi: Add initial OMAP5/DRA7x HDMI core IP library
From: Archit Taneja @ 2013-10-17 11:38 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1382009202-18984-1-git-send-email-archit@ti.com>

Add the OMAP5 HDMI core IP library. This will used by the hdmi5 driver to
configure the HDMI core sub block found on OMAP5/DRA7x.

The new HDMI core IP consists of the video composing sub blocks: video sampler,
color space converter, video packetizer and frame composer. These are connected
to each other serially. There is an i2c/ddc master sub block which is
responsible for reading EDID info.

Each of the video sub block is programmed in hdmi5_configure. Currenlty, only the
minimal features for each sub block are exposed. We can configure the core only
in 24 bit color mode. As we add more features, the register programming to
configure the serial sub blocks will increase in complexity.

The register configurations for enabling and clearing interrupts are provided,
but interrupts themselves aren't used at the moment. One place where they will
come in use to read the hdmi ddc block's register which recieves a byte of data
from the slave. We currently just read the register after every millisecond.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/hdmi5_core.c | 812 +++++++++++++++++++++++++++++++++++
 drivers/video/omap2/dss/hdmi5_core.h | 298 +++++++++++++
 2 files changed, 1110 insertions(+)
 create mode 100644 drivers/video/omap2/dss/hdmi5_core.c
 create mode 100644 drivers/video/omap2/dss/hdmi5_core.h

diff --git a/drivers/video/omap2/dss/hdmi5_core.c b/drivers/video/omap2/dss/hdmi5_core.c
new file mode 100644
index 0000000..c9c0ccc
--- /dev/null
+++ b/drivers/video/omap2/dss/hdmi5_core.c
@@ -0,0 +1,812 @@
+/*
+ * OMAP5 HDMI CORE IP driver Library
+ * Copyright (C) 2013 Texas Instruments Incorporated
+ * Author: Mythri pk <mythripk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/string.h>
+#include <linux/seq_file.h>
+#include <drm/drm_edid.h>
+#if defined(CONFIG_OMAP5_DSS_HDMI_AUDIO)
+#include <sound/asound.h>
+#include <sound/asoundef.h>
+#endif
+
+#include "hdmi5_core.h"
+
+/* only 24 bit color depth used for now */
+static const struct csc_table csc_table_deepcolor[] = {
+	/* HDMI_DEEP_COLOR_24BIT */
+	[0] = { 7036, 0, 0, 32, 0, 7036, 0, 32, 0, 0, 7036, 32, },
+	/* HDMI_DEEP_COLOR_30BIT */
+	[1] = { 7015, 0, 0, 128, 0, 7015, 0, 128, 0, 0, 7015, 128, },
+	/* HDMI_DEEP_COLOR_36BIT */
+	[2] = { 7010, 0, 0, 512, 0, 7010, 0, 512, 0, 0, 7010, 512, },
+	/* FULL RANGE */
+	[3] = { 8192, 0, 0, 0, 0, 8192, 0, 0, 0, 0, 8192, 0, },
+};
+
+static void hdmi_core_ddc_req_addr(struct hdmi_core_data *core, u8 addr, u8 ext)
+{
+	void __iomem *base = core->base;
+	u8 seg_ptr = ext / 2;
+	u8 edidaddr = ((ext % 2) * 0x80) + addr;
+
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_ADDRESS, edidaddr, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_SEGPTR, seg_ptr, 7, 0);
+
+	if (seg_ptr)
+		REG_FLD_MOD(base, HDMI_CORE_I2CM_OPERATION, 1, 1, 1);
+	else
+		REG_FLD_MOD(base, HDMI_CORE_I2CM_OPERATION, 1, 0, 0);
+}
+
+static void hdmi_core_ddc_init(struct hdmi_core_data *core)
+{
+	void __iomem *base = core->base;
+
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_SDA_HOLD_ADDR, 0x18, 7, 0);
+
+	/* mask the i2c interrupts */
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x0, 2, 2);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x0, 6, 6);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_INT, 0x0, 2, 2);
+
+	/* Master clock division */
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_DIV, 0x5, 3, 0);
+
+	/* Standard speed counter */
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_SS_SCL_HCNT_1_ADDR, 0x00, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_SS_SCL_HCNT_0_ADDR, 0x6c, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_SS_SCL_LCNT_1_ADDR, 0x00, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_SS_SCL_LCNT_0_ADDR, 0x7f, 7, 0);
+
+	/* Fast speed counter */
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_FS_SCL_HCNT_1_ADDR, 0x0, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_FS_SCL_HCNT_0_ADDR, 0x0f, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_FS_SCL_LCNT_1_ADDR, 0x0, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_FS_SCL_LCNT_0_ADDR, 0x21, 7, 0);
+
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_SLAVE, 0x50, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_SEGADDR, 0x30, 6, 0);
+}
+
+static int hdmi_core_ddc_edid(struct hdmi_core_data *core, u8 *pedid, u8 ext)
+{
+	void __iomem *base = core->base;
+	u8 cur_addr = 0;
+
+	hdmi_core_ddc_req_addr(core, cur_addr, ext);
+
+	/* unmask the i2c interrupts */
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x1, 2, 2);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x1, 6, 6);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_INT, 0x1, 2, 2);
+
+	/*
+	 * FIXME: We read 128 bytes of data here with a msleep. Ideally, we
+	 * should read based on a completion interrupt, the completion
+	 * interrupt doesn't seem to work, so we just sleep for around a
+	 * millisecond instead.
+	 */
+	while (cur_addr < 128) {
+		usleep_range(1000, 1500);
+		pedid[cur_addr++] = REG_GET(base, HDMI_CORE_I2CM_DATAI, 7, 0);
+		hdmi_core_ddc_req_addr(core, cur_addr, ext);
+	}
+
+	return 0;
+}
+
+int hdmi5_read_edid(struct hdmi_core_data *core, u8 *edid, int len)
+{
+	int r, n, i;
+	int max_ext_blocks = (len / 128) - 1;
+
+	if (len < 128)
+		return -EINVAL;
+
+	hdmi_core_ddc_init(core);
+
+	r = hdmi_core_ddc_edid(core, edid, 0);
+	if (r) {
+		return r;
+	} else {
+		n = edid[0x7e];
+
+		if (n > max_ext_blocks)
+			n = max_ext_blocks;
+
+		for (i = 1; i <= n; i++) {
+			r = hdmi_core_ddc_edid(core, edid + i * EDID_LENGTH, i);
+			if (r)
+				return r;
+		}
+	}
+
+	return 0;
+}
+
+void hdmi5_core_dump(struct hdmi_core_data *core, struct seq_file *s)
+{
+
+#define DUMPCORE(r) seq_printf(s, "%-35s %08x\n", #r,\
+		hdmi_read_reg(core->base, r))
+
+	DUMPCORE(HDMI_CORE_FC_INVIDCONF);
+	DUMPCORE(HDMI_CORE_FC_INHACTIV0);
+	DUMPCORE(HDMI_CORE_FC_INHACTIV1);
+	DUMPCORE(HDMI_CORE_FC_INHBLANK0);
+	DUMPCORE(HDMI_CORE_FC_INHBLANK1);
+	DUMPCORE(HDMI_CORE_FC_INVACTIV0);
+	DUMPCORE(HDMI_CORE_FC_INVACTIV1);
+	DUMPCORE(HDMI_CORE_FC_INVBLANK);
+	DUMPCORE(HDMI_CORE_FC_HSYNCINDELAY0);
+	DUMPCORE(HDMI_CORE_FC_HSYNCINDELAY1);
+	DUMPCORE(HDMI_CORE_FC_HSYNCINWIDTH0);
+	DUMPCORE(HDMI_CORE_FC_HSYNCINWIDTH1);
+	DUMPCORE(HDMI_CORE_FC_VSYNCINDELAY);
+	DUMPCORE(HDMI_CORE_FC_VSYNCINWIDTH);
+	DUMPCORE(HDMI_CORE_FC_CTRLDUR);
+	DUMPCORE(HDMI_CORE_FC_EXCTRLDUR);
+	DUMPCORE(HDMI_CORE_FC_EXCTRLSPAC);
+	DUMPCORE(HDMI_CORE_FC_CH0PREAM);
+	DUMPCORE(HDMI_CORE_FC_CH1PREAM);
+	DUMPCORE(HDMI_CORE_FC_CH2PREAM);
+	DUMPCORE(HDMI_CORE_FC_AVICONF0);
+	DUMPCORE(HDMI_CORE_FC_AVICONF1);
+	DUMPCORE(HDMI_CORE_FC_AVICONF2);
+	DUMPCORE(HDMI_CORE_FC_AVIVID);
+	DUMPCORE(HDMI_CORE_FC_PRCONF);
+
+	DUMPCORE(HDMI_CORE_MC_CLKDIS);
+	DUMPCORE(HDMI_CORE_MC_SWRSTZREQ);
+	DUMPCORE(HDMI_CORE_MC_FLOWCTRL);
+	DUMPCORE(HDMI_CORE_MC_PHYRSTZ);
+	DUMPCORE(HDMI_CORE_MC_LOCKONCLOCK);
+
+	DUMPCORE(HDMI_CORE_I2CM_SLAVE);
+	DUMPCORE(HDMI_CORE_I2CM_ADDRESS);
+	DUMPCORE(HDMI_CORE_I2CM_DATAO);
+	DUMPCORE(HDMI_CORE_I2CM_DATAI);
+	DUMPCORE(HDMI_CORE_I2CM_OPERATION);
+	DUMPCORE(HDMI_CORE_I2CM_INT);
+	DUMPCORE(HDMI_CORE_I2CM_CTLINT);
+	DUMPCORE(HDMI_CORE_I2CM_DIV);
+	DUMPCORE(HDMI_CORE_I2CM_SEGADDR);
+	DUMPCORE(HDMI_CORE_I2CM_SOFTRSTZ);
+	DUMPCORE(HDMI_CORE_I2CM_SEGPTR);
+	DUMPCORE(HDMI_CORE_I2CM_SS_SCL_HCNT_1_ADDR);
+	DUMPCORE(HDMI_CORE_I2CM_SS_SCL_HCNT_0_ADDR);
+	DUMPCORE(HDMI_CORE_I2CM_SS_SCL_LCNT_1_ADDR);
+	DUMPCORE(HDMI_CORE_I2CM_SS_SCL_LCNT_0_ADDR);
+	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_HCNT_1_ADDR);
+	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_HCNT_0_ADDR);
+	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_LCNT_1_ADDR);
+	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_LCNT_0_ADDR);
+
+#undef DUMPCORE
+}
+
+static void hdmi_core_init(struct hdmi_core_vid_config *video_cfg,
+			struct hdmi_core_infoframe_avi *avi_cfg,
+			struct hdmi_config *cfg)
+{
+	DSSDBG("hdmi_core_init\n");
+
+	/* video core */
+	video_cfg->data_enable_pol = 1; /* It is always 1*/
+	video_cfg->v_fc_config.timings.hsync_level = cfg->timings.hsync_level;
+	video_cfg->v_fc_config.timings.x_res = cfg->timings.x_res;
+	video_cfg->v_fc_config.timings.hsw = cfg->timings.hsw - 1;
+	video_cfg->v_fc_config.timings.hbp = cfg->timings.hbp;
+	video_cfg->v_fc_config.timings.hfp = cfg->timings.hfp;
+	video_cfg->hblank = cfg->timings.hfp +
+				cfg->timings.hbp + cfg->timings.hsw - 1;
+	video_cfg->v_fc_config.timings.vsync_level = cfg->timings.vsync_level;
+	video_cfg->v_fc_config.timings.y_res = cfg->timings.y_res;
+	video_cfg->v_fc_config.timings.vsw = cfg->timings.vsw;
+	video_cfg->v_fc_config.timings.vfp = cfg->timings.vfp;
+	video_cfg->v_fc_config.timings.vbp = cfg->timings.vbp;
+	video_cfg->vblank_osc = 0; /* Always 0 - need to confirm */
+	video_cfg->vblank = cfg->timings.vsw +
+				cfg->timings.vfp + cfg->timings.vbp;
+	video_cfg->v_fc_config.cm.mode = cfg->cm.mode;
+	video_cfg->v_fc_config.timings.interlace = cfg->timings.interlace;
+
+	/* info frame */
+	avi_cfg->db1_format = 0;
+	avi_cfg->db1_active_info = 0;
+	avi_cfg->db1_bar_info_dv = 0;
+	avi_cfg->db1_scan_info = 0;
+	avi_cfg->db2_colorimetry = 0;
+	avi_cfg->db2_aspect_ratio = 0;
+	avi_cfg->db2_active_fmt_ar = 0;
+	avi_cfg->db3_itc = 0;
+	avi_cfg->db3_ec = 0;
+	avi_cfg->db3_q_range = 0;
+	avi_cfg->db3_nup_scaling = 0;
+	avi_cfg->db4_videocode = 0;
+	avi_cfg->db5_pixel_repeat = 0;
+	avi_cfg->db6_7_line_eoftop = 0;
+	avi_cfg->db8_9_line_sofbottom = 0;
+	avi_cfg->db10_11_pixel_eofleft = 0;
+	avi_cfg->db12_13_pixel_sofright = 0;
+}
+
+static void hdmi_core_config_fc(struct hdmi_core_data *core,
+		struct hdmi_core_vid_config *cfg)
+{
+	void __iomem *base = core->base;
+	unsigned char r = 0;
+	bool vsync_pol, hsync_pol;
+
+	vsync_pol +		cfg->v_fc_config.timings.vsync_level = OMAPDSS_SIG_ACTIVE_HIGH;
+	hsync_pol +		cfg->v_fc_config.timings.hsync_level = OMAPDSS_SIG_ACTIVE_HIGH;
+
+	/* Set hsync, vsync and data-enable polarity  */
+	r = hdmi_read_reg(base, HDMI_CORE_FC_INVIDCONF);
+	r = FLD_MOD(r, vsync_pol, 6, 6);
+	r = FLD_MOD(r, hsync_pol, 5, 5);
+	r = FLD_MOD(r, cfg->data_enable_pol, 4, 4);
+	r = FLD_MOD(r, cfg->vblank_osc, 1, 1);
+	r = FLD_MOD(r, cfg->v_fc_config.timings.interlace, 0, 0);
+	hdmi_write_reg(base, HDMI_CORE_FC_INVIDCONF, r);
+
+	/* set x resolution */
+	REG_FLD_MOD(base, HDMI_CORE_FC_INHACTIV1,
+			cfg->v_fc_config.timings.x_res >> 8, 4, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_INHACTIV0,
+			cfg->v_fc_config.timings.x_res & 0xff, 7, 0);
+
+	/* set y resolution */
+	REG_FLD_MOD(base, HDMI_CORE_FC_INVACTIV1,
+			cfg->v_fc_config.timings.y_res >> 8, 4, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_INVACTIV0,
+			cfg->v_fc_config.timings.y_res & 0xff, 7, 0);
+
+	/* set horizontal blanking pixels */
+	REG_FLD_MOD(base, HDMI_CORE_FC_INHBLANK1, cfg->hblank >> 8, 4, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_INHBLANK0, cfg->hblank & 0xff, 7, 0);
+
+	/* set vertial blanking pixels */
+	REG_FLD_MOD(base, HDMI_CORE_FC_INVBLANK, cfg->vblank, 7, 0);
+
+	/* set horizontal sync offset */
+	REG_FLD_MOD(base, HDMI_CORE_FC_HSYNCINDELAY1,
+			cfg->v_fc_config.timings.hfp >> 8, 4, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_HSYNCINDELAY0,
+			cfg->v_fc_config.timings.hfp & 0xff, 7, 0);
+
+	/* set vertical sync offset */
+	REG_FLD_MOD(base, HDMI_CORE_FC_VSYNCINDELAY,
+			cfg->v_fc_config.timings.vfp, 7, 0);
+
+	/* set horizontal sync pulse width */
+	REG_FLD_MOD(base, HDMI_CORE_FC_HSYNCINWIDTH1,
+			(cfg->v_fc_config.timings.hsw >> 8), 1, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_HSYNCINWIDTH0,
+			cfg->v_fc_config.timings.hsw & 0xff, 7, 0);
+
+	/*  set vertical sync pulse width */
+	REG_FLD_MOD(base, HDMI_CORE_FC_VSYNCINWIDTH,
+			cfg->v_fc_config.timings.vsw, 5, 0);
+
+	/* select DVI mode */
+	REG_FLD_MOD(base, HDMI_CORE_FC_INVIDCONF,
+			cfg->v_fc_config.cm.mode, 3, 3);
+}
+
+static void hdmi_core_config_video_packetizer(struct hdmi_core_data *core)
+{
+	void __iomem *base = core->base;
+	int clr_depth = 0;	/* 24 bit color depth */
+
+	/* COLOR_DEPTH */
+	REG_FLD_MOD(base, HDMI_CORE_VP_PR_CD, clr_depth, 7, 4);
+	/* BYPASS_EN */
+	REG_FLD_MOD(base, HDMI_CORE_VP_CONF, clr_depth ? 0 : 1, 6, 6);
+	/* PP_EN */
+	REG_FLD_MOD(base, HDMI_CORE_VP_CONF, clr_depth ? 1 : 0, 5, 5);
+	/* YCC422_EN */
+	REG_FLD_MOD(base, HDMI_CORE_VP_CONF, 0, 3, 3);
+	/* PP_STUFFING */
+	REG_FLD_MOD(base, HDMI_CORE_VP_STUFF, clr_depth ? 1 : 0, 1, 1);
+	/* YCC422_STUFFING */
+	REG_FLD_MOD(base, HDMI_CORE_VP_STUFF, 1, 2, 2);
+	/* OUTPUT_SELECTOR */
+	REG_FLD_MOD(base, HDMI_CORE_VP_CONF, clr_depth ? 0 : 2, 1, 0);
+}
+
+static void hdmi_core_config_csc(struct hdmi_core_data *core)
+{
+	void __iomem *base = core->base;
+	int clr_depth;
+	struct csc_table csc_coeff = { 0 };
+
+	/* support limited range with 24 bit color depth for now */
+	csc_coeff = csc_table_deepcolor[0];
+	clr_depth = 0;
+	core->avi_cfg.db3_q_range = HDMI_INFOFRAME_AVI_DB3Q_LR;
+
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A1_MSB, csc_coeff.a1 >> 8 , 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A1_LSB, csc_coeff.a1, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A2_MSB, csc_coeff.a2 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A2_LSB, csc_coeff.a2, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A3_MSB, csc_coeff.a3 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A3_LSB, csc_coeff.a3, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A4_MSB, csc_coeff.a4 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_A4_LSB, csc_coeff.a4, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B1_MSB, csc_coeff.b1 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B1_LSB, csc_coeff.b1, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B2_MSB, csc_coeff.b2 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B2_LSB, csc_coeff.b2, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B3_MSB, csc_coeff.b3 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B3_LSB, csc_coeff.b3, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B4_MSB, csc_coeff.b4 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_B4_LSB, csc_coeff.b4, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C1_MSB, csc_coeff.c1 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C1_LSB, csc_coeff.c1, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C2_MSB, csc_coeff.c2 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C2_LSB, csc_coeff.c2, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C3_MSB, csc_coeff.c3 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C3_LSB, csc_coeff.c3, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C4_MSB, csc_coeff.c4 >> 8, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CSC_COEF_C4_LSB, csc_coeff.c4, 7, 0);
+
+	/* CSC_FEED_THROUGH_OFF */
+	REG_FLD_MOD(base, HDMI_CORE_MC_FLOWCTRL, 0x1, 0, 0);
+
+	/* CSC_COLORDEPTH */
+	REG_FLD_MOD(base, HDMI_CORE_CSC_SCALE, clr_depth, 7, 4);
+}
+
+static void hdmi_core_config_video_sampler(struct hdmi_core_data *core)
+{
+	int video_mapping = 1;	/* for 24 bit color depth */
+
+	/* VIDEO_MAPPING */
+	REG_FLD_MOD(core->base, HDMI_CORE_TX_INVID0, video_mapping, 4, 0);
+}
+
+static void hdmi_core_config_fc_aux_infoframe_avi(struct hdmi_core_data *core)
+{
+	void __iomem *base = core->base;
+	struct hdmi_core_infoframe_avi avi = core->avi_cfg;
+
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF0, avi.db1_format, 1, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF0, avi.db1_active_info, 6, 6);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF0, avi.db1_bar_info_dv, 3, 2);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF0, avi.db1_scan_info, 5, 4);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF1, avi.db2_colorimetry, 7, 6);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF1, avi.db2_aspect_ratio, 5, 4);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF1, avi.db2_active_fmt_ar, 3, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF2, avi.db3_itc, 7, 7);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF2, avi.db3_ec, 6, 4);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF2, avi.db3_q_range, 3, 2);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVICONF2, avi.db3_nup_scaling, 1, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AVIVID, avi.db4_videocode, 6, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_PRCONF, avi.db5_pixel_repeat, 3, 0);
+}
+
+static void hdmi_core_enable_video_path(struct hdmi_core_data *core)
+{
+	void __iomem *base = core->base;
+
+	DSSDBG("hdmi_core_enable_video_path\n");
+
+	REG_FLD_MOD(base, HDMI_CORE_FC_CTRLDUR, 0x0C, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_EXCTRLDUR, 0x20, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_EXCTRLSPAC, 0x01, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_CH0PREAM, 0x0B, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_CH1PREAM, 0x16, 5, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_CH2PREAM, 0x21, 5, 0);
+	REG_FLD_MOD(base, HDMI_CORE_MC_CLKDIS, 0x00, 0, 0);
+	REG_FLD_MOD(base, HDMI_CORE_MC_CLKDIS, 0x00, 1, 1);
+}
+
+static void hdmi_core_mask_interrupts(struct hdmi_core_data *core)
+{
+	void __iomem *base = core->base;
+
+	REG_FLD_MOD(base, HDMI_CORE_VP_MASK, 0x0, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_MASK0, 0x0, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_MASK1, 0x0, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_MASK2, 0x0, 1, 0);
+	REG_FLD_MOD(base, HDMI_CORE_PHY_MASK0, 0x0, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_PHY_I2CM_INT_ADDR, 0x8, 3, 0);
+	REG_FLD_MOD(base, HDMI_CORE_PHY_I2CM_CTLINT_ADDR, 0x88, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_AUD_INT, 0xa3, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_AUD_CC08, 0x0, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_AUD_D010, 0x0, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_AUD_GP_MASK, 0x3, 1, 0);
+	REG_FLD_MOD(base, HDMI_CORE_HDCP_MASK, 0x0, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_CEC_MASK, 0xff, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_INT, 0x1, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0xff, 7, 0);
+}
+
+static void hdmi_core_enable_interrupts(struct hdmi_core_data *core)
+{
+	/* Unmute interrupts */
+	REG_FLD_MOD(core->base, HDMI_CORE_IH_MUTE, 0x0, 1, 0);
+}
+
+int hdmi5_core_handle_irqs(struct hdmi_core_data *core)
+{
+	void __iomem *base = core->base;
+
+	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT0, 0xff, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT1, 0xff, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT2, 0xff, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_IH_AS_STAT0, 0xff, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_IH_PHY_STAT0, 0xff, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_IH_I2CM_STAT0, 0xff, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_IH_CEC_STAT0, 0xff, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_IH_VP_STAT0, 0xff, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_IH_I2CMPHY_STAT0, 0xff, 7, 0);
+
+	return 0;
+}
+
+void hdmi5_configure(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
+		struct hdmi_config *cfg)
+{
+	struct omap_video_timings video_timing;
+	struct hdmi_video_format video_format;
+	struct hdmi_core_vid_config v_core_cfg;
+	struct hdmi_core_infoframe_avi *avi_cfg = &core->avi_cfg;
+
+	hdmi_core_mask_interrupts(core);
+
+	hdmi_core_init(&v_core_cfg, avi_cfg, cfg);
+
+	hdmi_wp_init_vid_fmt_timings(&video_format, &video_timing, cfg);
+
+	hdmi_wp_video_config_timing(wp, &video_timing);
+
+	/* video config */
+	video_format.packing_mode = HDMI_PACK_24b_RGB_YUV444_YUV422;
+
+	hdmi_wp_video_config_format(wp, &video_format);
+
+	hdmi_wp_video_config_interface(wp, &video_timing);
+
+	/*
+	 * configure core video path, set software reset in the core
+	 */
+	v_core_cfg.packet_mode = HDMI_PACKETMODE24BITPERPIXEL;
+
+	hdmi_core_config_fc(core, &v_core_cfg);
+	hdmi_core_config_video_packetizer(core);
+	hdmi_core_config_csc(core);
+	hdmi_core_config_video_sampler(core);
+
+	/*
+	 * configure packet info frame video according to CEA861-D
+	 */
+	avi_cfg->db1_format = HDMI_INFOFRAME_AVI_DB1Y_RGB;
+	avi_cfg->db1_active_info +			HDMI_INFOFRAME_AVI_DB1A_ACTIVE_FORMAT_OFF;
+	avi_cfg->db1_bar_info_dv = HDMI_INFOFRAME_AVI_DB1B_NO;
+	avi_cfg->db1_scan_info = HDMI_INFOFRAME_AVI_DB1S_0;
+	avi_cfg->db2_colorimetry = HDMI_INFOFRAME_AVI_DB2C_NO;
+	avi_cfg->db2_aspect_ratio = HDMI_INFOFRAME_AVI_DB2M_NO;
+	avi_cfg->db2_active_fmt_ar = HDMI_INFOFRAME_AVI_DB2R_SAME;
+	avi_cfg->db3_itc = HDMI_INFOFRAME_AVI_DB3ITC_NO;
+	avi_cfg->db3_ec = HDMI_INFOFRAME_AVI_DB3EC_XVYUV601;
+	avi_cfg->db3_q_range = HDMI_INFOFRAME_AVI_DB3Q_DEFAULT;
+	avi_cfg->db3_nup_scaling = HDMI_INFOFRAME_AVI_DB3SC_NO;
+	avi_cfg->db4_videocode = cfg->cm.code;
+	avi_cfg->db5_pixel_repeat = HDMI_INFOFRAME_AVI_DB5PR_NO;
+	avi_cfg->db6_7_line_eoftop = 0;
+	avi_cfg->db8_9_line_sofbottom = 0;
+	avi_cfg->db10_11_pixel_eofleft = 0;
+	avi_cfg->db12_13_pixel_sofright = 0;
+
+	hdmi_core_config_fc_aux_infoframe_avi(core);
+
+	hdmi_core_enable_video_path(core);
+
+	hdmi_core_enable_interrupts(core);
+}
+
+
+#if defined(CONFIG_OMAP5_DSS_HDMI_AUDIO)
+
+static void hdmi5_core_audio_config(struct hdmi_core_data *core,
+			struct hdmi_core_audio_config *cfg)
+{
+	void __iomem *base = core->base;
+	u8 val;
+
+	/* Mute audio before configuring */
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCONF, 0xf, 7, 4);
+
+	/* Set the N parameter */
+	REG_FLD_MOD(base, HDMI_CORE_AUD_N1, cfg->n, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_AUD_N2, cfg->n >> 8, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_AUD_N3, cfg->n >> 16, 3, 0);
+
+	/*
+	 * CTS manual mode. Automatic mode is not supported when using audio
+	 * parallel interface.
+	 */
+	REG_FLD_MOD(base, HDMI_CORE_AUD_CTS3, 1, 4, 4);
+	REG_FLD_MOD(base, HDMI_CORE_AUD_CTS1, cfg->cts, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_AUD_CTS2, cfg->cts >> 8, 7, 0);
+	REG_FLD_MOD(base, HDMI_CORE_AUD_CTS3, cfg->cts >> 16, 3, 0);
+
+	/* Layout of Audio Sample Packets: 2-channel or multichannels */
+	if (cfg->layout = HDMI_AUDIO_LAYOUT_2CH)
+		REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCONF, 0, 0, 0);
+	else
+		REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCONF, 1, 0, 0);
+
+	/* Configure IEC-609580 Validity bits */
+	/* Channel 0 is valid */
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSV, 0, 0, 0);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSV, 0, 4, 4);
+
+	if (cfg->layout = HDMI_AUDIO_LAYOUT_2CH)
+		val = 1;
+	else
+		val = 0;
+
+	/* Channels 1, 2 setting */
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSV, val, 1, 1);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSV, val, 5, 5);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSV, val, 2, 2);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSV, val, 6, 6);
+	/* Channel 3 setting */
+	if (cfg->layout = HDMI_AUDIO_LAYOUT_6CH)
+		val = 1;
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSV, val, 3, 3);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSV, val, 7, 7);
+
+	/* Configure IEC-60958 User bits */
+	/* TODO: should be set by user. */
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSU, 0, 7, 0);
+
+	/* Configure IEC-60958 Channel Status word */
+	/* CGMSA */
+	val = cfg->iec60958_cfg->status[5] & IEC958_AES5_CON_CGMSA;
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(0), val, 5, 4);
+
+	/* Copyright */
+	val = (cfg->iec60958_cfg->status[0] &
+			IEC958_AES0_CON_NOT_COPYRIGHT) >> 2;
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(0), val, 0, 0);
+
+	/* Category */
+	hdmi_write_reg(base, HDMI_CORE_FC_AUDSCHNLS(1),
+		cfg->iec60958_cfg->status[1]);
+
+	/* PCM audio mode */
+	val = (cfg->iec60958_cfg->status[0] & IEC958_AES0_CON_MODE) >> 6;
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 6, 4);
+
+	/* Source number */
+	val = cfg->iec60958_cfg->status[2] & IEC958_AES2_CON_SOURCE;
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 3, 4);
+
+	/* Channel number right 0  */
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(3), 2, 3, 0);
+	/* Channel number right 1*/
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(3), 4, 7, 4);
+	/* Channel number right 2  */
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(4), 6, 3, 0);
+	/* Channel number right 3*/
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(4), 8, 7, 4);
+	/* Channel number left 0  */
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(5), 1, 3, 0);
+	/* Channel number left 1*/
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(5), 3, 7, 4);
+	/* Channel number left 2  */
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(6), 5, 3, 0);
+	/* Channel number left 3*/
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(6), 7, 7, 4);
+
+	/* Clock accuracy and sample rate */
+	hdmi_write_reg(base, HDMI_CORE_FC_AUDSCHNLS(7),
+		cfg->iec60958_cfg->status[3]);
+
+	/* Original sample rate and word length */
+	hdmi_write_reg(base, HDMI_CORE_FC_AUDSCHNLS(8),
+		cfg->iec60958_cfg->status[4]);
+
+	/* Enable FIFO empty and full interrupts */
+	REG_FLD_MOD(base, HDMI_CORE_AUD_INT, 3, 3, 2);
+
+	/* Configure GPA */
+	/* select HBR/SPDIF interfaces */
+	if (cfg->layout = HDMI_AUDIO_LAYOUT_2CH) {
+		/* select HBR/SPDIF interfaces */
+		REG_FLD_MOD(base, HDMI_CORE_AUD_CONF0, 0, 5, 5);
+		/* enable two channels in GPA */
+		REG_FLD_MOD(base, HDMI_CORE_AUD_GP_CONF1, 3, 7, 0);
+	} else if (cfg->layout = HDMI_AUDIO_LAYOUT_6CH) {
+		/* select HBR/SPDIF interfaces */
+		REG_FLD_MOD(base, HDMI_CORE_AUD_CONF0, 0, 5, 5);
+		/* enable six channels in GPA */
+		REG_FLD_MOD(base, HDMI_CORE_AUD_GP_CONF1, 0x3f, 7, 0);
+	} else {
+		/* select HBR/SPDIF interfaces */
+		REG_FLD_MOD(base, HDMI_CORE_AUD_CONF0, 0, 5, 5);
+		/* enable eight channels in GPA */
+		REG_FLD_MOD(base, HDMI_CORE_AUD_GP_CONF1, 0xff, 7, 0);
+	}
+
+	/* disable HBR */
+	REG_FLD_MOD(base, HDMI_CORE_AUD_GP_CONF2, 0, 0, 0);
+	/* enable PCUV */
+	REG_FLD_MOD(base, HDMI_CORE_AUD_GP_CONF2, 1, 1, 1);
+	/* enable GPA FIFO full and empty mask */
+	REG_FLD_MOD(base, HDMI_CORE_AUD_GP_MASK, 3, 1, 0);
+	/* set polarity of GPA FIFO empty interrupts */
+	REG_FLD_MOD(base, HDMI_CORE_AUD_GP_POL, 1, 0, 0);
+
+	/* unmute audio */
+	REG_FLD_MOD(core_sys_base, HDMI_CORE_FC_AUDSCONF, 0, 7, 4);
+}
+
+static void hdmi5_core_audio_infoframe_cfg(struct hdmi_core_data *core,
+	 struct snd_cea_861_aud_if *info_aud)
+{
+	void __iomem *base = core->base;
+
+	/* channel count and coding type fields in AUDICONF0 are swapped */
+	hdmi_write_reg(base, HDMI_CORE_FC_AUDICONF0,
+		(info_aud->db1_ct_cc & CEA861_AUDIO_INFOFRAME_DB1CC) << 4 |
+		(info_aud->db1_ct_cc & CEA861_AUDIO_INFOFRAME_DB1CT) >> 4);
+
+	hdmi_write_reg(base, HDMI_CORE_FC_AUDICONF1, info_aud->db2_sf_ss);
+	hdmi_write_reg(base, HDMI_CORE_FC_AUDICONF2, info_aud->db4_ca);
+	hdmi_write_reg(base, HDMI_CORE_FC_AUDICONF3, info_aud->db5_dminh_lsv);
+}
+
+int hdmi5_audio_config(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
+			struct omap_dss_audio *audio, u32 pclk)
+{
+	struct hdmi_audio_format audio_format;
+	struct hdmi_audio_dma audio_dma;
+	struct hdmi_core_audio_config core_cfg;
+	int err, n, cts, channel_count;
+	unsigned int fs_nr;
+	bool word_length_16b = false;
+
+	if (!audio || !audio->iec || !audio->cea || !core)
+		return -EINVAL;
+
+	core_cfg.iec60958_cfg = audio->iec;
+
+	if (!(audio->iec->status[4] & IEC958_AES4_CON_MAX_WORDLEN_24) &&
+		(audio->iec->status[4] & IEC958_AES4_CON_WORDLEN_20_16))
+			word_length_16b = true;
+
+	/* only 16-bit word length supported atm */
+	if (!word_length_16b)
+		return -EINVAL;
+
+	switch (audio->iec->status[3] & IEC958_AES3_CON_FS) {
+	case IEC958_AES3_CON_FS_32000:
+		fs_nr = 32000;
+		break;
+	case IEC958_AES3_CON_FS_44100:
+		fs_nr = 44100;
+		break;
+	case IEC958_AES3_CON_FS_48000:
+		fs_nr = 48000;
+		break;
+	case IEC958_AES3_CON_FS_88200:
+		fs_nr = 88200;
+		break;
+	case IEC958_AES3_CON_FS_96000:
+		fs_nr = 96000;
+		break;
+	case IEC958_AES3_CON_FS_176400:
+		fs_nr = 176400;
+		break;
+	case IEC958_AES3_CON_FS_192000:
+		fs_nr = 192000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	err = hdmi_compute_acr(pclk, fs_nr, &n, &cts);
+	core_cfg.n = n;
+	core_cfg.cts = cts;
+
+	/* Audio channels settings */
+	channel_count = (audio->cea->db1_ct_cc & CEA861_AUDIO_INFOFRAME_DB1CC)
+				+ 1;
+
+	if (channel_count = 2)
+		core_cfg.layout = HDMI_AUDIO_LAYOUT_2CH;
+	else if (channel_count = 6)
+		core_cfg.layout = HDMI_AUDIO_LAYOUT_6CH;
+	else
+		core_cfg.layout = HDMI_AUDIO_LAYOUT_8CH;
+
+	/* DMA settings */
+	if (word_length_16b)
+		audio_dma.transfer_size = 0x10;
+	else
+		audio_dma.transfer_size = 0x20;
+	audio_dma.block_size = 0xc0;
+	audio_dma.mode = HDMI_AUDIO_TRANSF_DMA;
+	audio_dma.fifo_threshold = 0x20; /* in number of samples */
+
+	/* audio FIFO format settings for 16-bit samples*/
+	audio_format.samples_per_word = HDMI_AUDIO_ONEWORD_TWOSAMPLES;
+	audio_format.sample_size = HDMI_AUDIO_SAMPLE_16BITS;
+	audio_format.justification = HDMI_AUDIO_JUSTIFY_LEFT;
+
+	/* only LPCM atm */
+	audio_format.type = HDMI_AUDIO_TYPE_LPCM;
+
+	/* disable start/stop signals of IEC 60958 blocks */
+	audio_format.en_sig_blk_strt_end = HDMI_AUDIO_BLOCK_SIG_STARTEND_ON;
+
+	/* configure DMA and audio FIFO format*/
+	hdmi_wp_audio_config_dma(wp, &audio_dma);
+	hdmi_wp_audio_config_format(wp, &audio_format);
+
+	/* configure the core */
+	hdmi5_core_audio_config(core, &core_cfg);
+
+	/* configure CEA 861 audio infoframe */
+	hdmi5_core_audio_infoframe_cfg(core, audio->cea);
+
+	return 0;
+}
+#endif
+
+#define CORE_OFFSET		0x20000
+#define CORE_SIZE		0x17900
+
+int hdmi5_core_init(struct platform_device *pdev, struct hdmi_core_data *core)
+{
+	struct resource *res;
+	struct resource temp_res;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hdmi_core");
+	if (!res) {
+		DSSDBG("can't get CORE mem resource by name\n");
+		/*
+		 * if hwmod/DT doesn't have the memory resource information
+		 * split into HDMI sub blocks by name, we try again by getting
+		 * the platform's first resource. this code will be removed when
+		 * the driver can get the mem resources by name
+		 */
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		if (!res) {
+			DSSERR("can't get CORE mem resource\n");
+			return -EINVAL;
+		}
+
+		temp_res.start = res->start + CORE_OFFSET;
+		temp_res.end = temp_res.start + CORE_SIZE - 1;
+		res = &temp_res;
+	}
+
+	core->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!core->base) {
+		DSSERR("can't ioremap CORE\n");
+		return -ENOMEM;
+	}
+
+	return 0;
+}
diff --git a/drivers/video/omap2/dss/hdmi5_core.h b/drivers/video/omap2/dss/hdmi5_core.h
new file mode 100644
index 0000000..26def54
--- /dev/null
+++ b/drivers/video/omap2/dss/hdmi5_core.h
@@ -0,0 +1,298 @@
+/*
+ * HDMI driver definition for TI OMAP5 processors.
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef _HDMI5_CORE_H_
+#define _HDMI5_CORE_H_
+
+#include "hdmi.h"
+
+/* HDMI IP Core System */
+
+/* HDMI Identification */
+#define HDMI_CORE_DESIGN_ID			0x00000
+#define HDMI_CORE_REVISION_ID			0x00004
+#define HDMI_CORE_PRODUCT_ID0			0x00008
+#define HDMI_CORE_PRODUCT_ID1			0x0000C
+#define HDMI_CORE_CONFIG0_ID			0x00010
+#define HDMI_CORE_CONFIG1_ID			0x00014
+#define HDMI_CORE_CONFIG2_ID			0x00018
+#define HDMI_CORE_CONFIG3_ID			0x0001C
+
+/* HDMI Interrupt */
+#define HDMI_CORE_IH_FC_STAT0			0x00400
+#define HDMI_CORE_IH_FC_STAT1			0x00404
+#define HDMI_CORE_IH_FC_STAT2			0x00408
+#define HDMI_CORE_IH_AS_STAT0			0x0040C
+#define HDMI_CORE_IH_PHY_STAT0			0x00410
+#define HDMI_CORE_IH_I2CM_STAT0			0x00414
+#define HDMI_CORE_IH_CEC_STAT0			0x00418
+#define HDMI_CORE_IH_VP_STAT0			0x0041C
+#define HDMI_CORE_IH_I2CMPHY_STAT0		0x00420
+#define HDMI_CORE_IH_MUTE			0x007FC
+
+/* HDMI Video Sampler */
+#define HDMI_CORE_TX_INVID0			0x00800
+#define HDMI_CORE_TX_INSTUFFING			0x00804
+#define HDMI_CORE_TX_RGYDATA0			0x00808
+#define HDMI_CORE_TX_RGYDATA1			0x0080C
+#define HDMI_CORE_TX_RCRDATA0			0x00810
+#define HDMI_CORE_TX_RCRDATA1			0x00814
+#define HDMI_CORE_TX_BCBDATA0			0x00818
+#define HDMI_CORE_TX_BCBDATA1			0x0081C
+
+/* HDMI Video Packetizer */
+#define HDMI_CORE_VP_STATUS			0x02000
+#define HDMI_CORE_VP_PR_CD			0x02004
+#define HDMI_CORE_VP_STUFF			0x02008
+#define HDMI_CORE_VP_REMAP			0x0200C
+#define HDMI_CORE_VP_CONF			0x02010
+#define HDMI_CORE_VP_STAT			0x02014
+#define HDMI_CORE_VP_INT			0x02018
+#define HDMI_CORE_VP_MASK			0x0201C
+#define HDMI_CORE_VP_POL			0x02020
+
+/* Frame Composer */
+#define HDMI_CORE_FC_INVIDCONF			0x04000
+#define HDMI_CORE_FC_INHACTIV0			0x04004
+#define HDMI_CORE_FC_INHACTIV1			0x04008
+#define HDMI_CORE_FC_INHBLANK0			0x0400C
+#define HDMI_CORE_FC_INHBLANK1			0x04010
+#define HDMI_CORE_FC_INVACTIV0			0x04014
+#define HDMI_CORE_FC_INVACTIV1			0x04018
+#define HDMI_CORE_FC_INVBLANK			0x0401C
+#define HDMI_CORE_FC_HSYNCINDELAY0		0x04020
+#define HDMI_CORE_FC_HSYNCINDELAY1		0x04024
+#define HDMI_CORE_FC_HSYNCINWIDTH0		0x04028
+#define HDMI_CORE_FC_HSYNCINWIDTH1		0x0402C
+#define HDMI_CORE_FC_VSYNCINDELAY		0x04030
+#define HDMI_CORE_FC_VSYNCINWIDTH		0x04034
+#define HDMI_CORE_FC_INFREQ0			0x04038
+#define HDMI_CORE_FC_INFREQ1			0x0403C
+#define HDMI_CORE_FC_INFREQ2			0x04040
+#define HDMI_CORE_FC_CTRLDUR			0x04044
+#define HDMI_CORE_FC_EXCTRLDUR			0x04048
+#define HDMI_CORE_FC_EXCTRLSPAC			0x0404C
+#define HDMI_CORE_FC_CH0PREAM			0x04050
+#define HDMI_CORE_FC_CH1PREAM			0x04054
+#define HDMI_CORE_FC_CH2PREAM			0x04058
+#define HDMI_CORE_FC_AVICONF3			0x0405C
+#define HDMI_CORE_FC_GCP			0x04060
+#define HDMI_CORE_FC_AVICONF0			0x04064
+#define HDMI_CORE_FC_AVICONF1			0x04068
+#define HDMI_CORE_FC_AVICONF2			0x0406C
+#define HDMI_CORE_FC_AVIVID			0x04070
+#define HDMI_CORE_FC_AVIETB0			0x04074
+#define HDMI_CORE_FC_AVIETB1			0x04078
+#define HDMI_CORE_FC_AVISBB0			0x0407C
+#define HDMI_CORE_FC_AVISBB1			0x04080
+#define HDMI_CORE_FC_AVIELB0			0x04084
+#define HDMI_CORE_FC_AVIELB1			0x04088
+#define HDMI_CORE_FC_AVISRB0			0x0408C
+#define HDMI_CORE_FC_AVISRB1			0x04090
+#define HDMI_CORE_FC_AUDICONF0			0x04094
+#define HDMI_CORE_FC_AUDICONF1			0x04098
+#define HDMI_CORE_FC_AUDICONF2			0x0409C
+#define HDMI_CORE_FC_AUDICONF3			0x040A0
+#define HDMI_CORE_FC_VSDIEEEID0			0x040A4
+#define HDMI_CORE_FC_VSDSIZE			0x040A8
+#define HDMI_CORE_FC_VSDIEEEID1			0x040C0
+#define HDMI_CORE_FC_VSDIEEEID2			0x040C4
+#define HDMI_CORE_FC_VSDPAYLOAD(n)		(n * 4 + 0x040C8)
+#define HDMI_CORE_FC_SPDVENDORNAME(n)		(n * 4 + 0x04128)
+#define HDMI_CORE_FC_SPDPRODUCTNAME(n)		(n * 4 + 0x04148)
+#define HDMI_CORE_FC_SPDDEVICEINF		0x04188
+#define HDMI_CORE_FC_AUDSCONF			0x0418C
+#define HDMI_CORE_FC_AUDSSTAT			0x04190
+#define HDMI_CORE_FC_AUDSV			0x04194
+#define HDMI_CORE_FC_AUDSU			0x04198
+#define HDMI_CORE_FC_AUDSCHNLS(n)		(n * 4 + 0x0419C)
+#define HDMI_CORE_FC_CTRLQHIGH			0x041CC
+#define HDMI_CORE_FC_CTRLQLOW			0x041D0
+#define HDMI_CORE_FC_ACP0			0x041D4
+#define HDMI_CORE_FC_ACP(n)			((16-n) * 4 + 0x04208)
+#define HDMI_CORE_FC_ISCR1_0			0x04248
+#define HDMI_CORE_FC_ISCR1(n)			((16-n) * 4 + 0x0424C)
+#define HDMI_CORE_FC_ISCR2(n)			((15-n) * 4 + 0x0428C)
+#define HDMI_CORE_FC_DATAUTO0			0x042CC
+#define HDMI_CORE_FC_DATAUTO1			0x042D0
+#define HDMI_CORE_FC_DATAUTO2			0x042D4
+#define HDMI_CORE_FC_DATMAN			0x042D8
+#define HDMI_CORE_FC_DATAUTO3			0x042DC
+#define HDMI_CORE_FC_RDRB(n)			(n * 4 + 0x042E0)
+#define HDMI_CORE_FC_STAT0			0x04340
+#define HDMI_CORE_FC_INT0			0x04344
+#define HDMI_CORE_FC_MASK0			0x04348
+#define HDMI_CORE_FC_POL0			0x0434C
+#define HDMI_CORE_FC_STAT1			0x04350
+#define HDMI_CORE_FC_INT1			0x04354
+#define HDMI_CORE_FC_MASK1			0x04358
+#define HDMI_CORE_FC_POL1			0x0435C
+#define HDMI_CORE_FC_STAT2			0x04360
+#define HDMI_CORE_FC_INT2			0x04364
+#define HDMI_CORE_FC_MASK2			0x04368
+#define HDMI_CORE_FC_POL2			0x0436C
+#define HDMI_CORE_FC_PRCONF			0x04380
+#define HDMI_CORE_FC_GMD_STAT			0x04400
+#define HDMI_CORE_FC_GMD_EN			0x04404
+#define HDMI_CORE_FC_GMD_UP			0x04408
+#define HDMI_CORE_FC_GMD_CONF			0x0440C
+#define HDMI_CORE_FC_GMD_HB			0x04410
+#define HDMI_CORE_FC_GMD_PB(n)			(n * 4 + 0x04414)
+#define HDMI_CORE_FC_DBGFORCE			0x04800
+#define HDMI_CORE_FC_DBGAUD0CH0			0x04804
+#define HDMI_CORE_FC_DBGAUD1CH0			0x04808
+#define HDMI_CORE_FC_DBGAUD2CH0			0x0480C
+#define HDMI_CORE_FC_DBGAUD0CH1			0x04810
+#define HDMI_CORE_FC_DBGAUD1CH1			0x04814
+#define HDMI_CORE_FC_DBGAUD2CH1			0x04818
+#define HDMI_CORE_FC_DBGAUD0CH2			0x0481C
+#define HDMI_CORE_FC_DBGAUD1CH2			0x04820
+#define HDMI_CORE_FC_DBGAUD2CH2			0x04824
+#define HDMI_CORE_FC_DBGAUD0CH3			0x04828
+#define HDMI_CORE_FC_DBGAUD1CH3			0x0482C
+#define HDMI_CORE_FC_DBGAUD2CH3			0x04830
+#define HDMI_CORE_FC_DBGAUD0CH4			0x04834
+#define HDMI_CORE_FC_DBGAUD1CH4			0x04838
+#define HDMI_CORE_FC_DBGAUD2CH4			0x0483C
+#define HDMI_CORE_FC_DBGAUD0CH5			0x04840
+#define HDMI_CORE_FC_DBGAUD1CH5			0x04844
+#define HDMI_CORE_FC_DBGAUD2CH5			0x04848
+#define HDMI_CORE_FC_DBGAUD0CH6			0x0484C
+#define HDMI_CORE_FC_DBGAUD1CH6			0x04850
+#define HDMI_CORE_FC_DBGAUD2CH6			0x04854
+#define HDMI_CORE_FC_DBGAUD0CH7			0x04858
+#define HDMI_CORE_FC_DBGAUD1CH7			0x0485C
+#define HDMI_CORE_FC_DBGAUD2CH7			0x04860
+#define HDMI_CORE_FC_DBGTMDS0			0x04864
+#define HDMI_CORE_FC_DBGTMDS1			0x04868
+#define HDMI_CORE_FC_DBGTMDS2			0x0486C
+#define HDMI_CORE_PHY_MASK0			0x0C018
+#define HDMI_CORE_PHY_I2CM_INT_ADDR		0x0C09C
+#define HDMI_CORE_PHY_I2CM_CTLINT_ADDR		0x0C0A0
+
+/* HDMI Audio */
+#define HDMI_CORE_AUD_CONF0			0x0C400
+#define HDMI_CORE_AUD_CONF1			0x0C404
+#define HDMI_CORE_AUD_INT			0x0C408
+#define HDMI_CORE_AUD_N1			0x0C800
+#define HDMI_CORE_AUD_N2			0x0C804
+#define HDMI_CORE_AUD_N3			0x0C808
+#define HDMI_CORE_AUD_CTS1			0x0C80C
+#define HDMI_CORE_AUD_CTS2			0x0C810
+#define HDMI_CORE_AUD_CTS3			0x0C814
+#define HDMI_CORE_AUD_INCLKFS			0x0C818
+#define HDMI_CORE_AUD_CC08			0x0CC08
+#define HDMI_CORE_AUD_GP_CONF0			0x0D400
+#define HDMI_CORE_AUD_GP_CONF1			0x0D404
+#define HDMI_CORE_AUD_GP_CONF2			0x0D408
+#define HDMI_CORE_AUD_D010			0x0D010
+#define HDMI_CORE_AUD_GP_STAT			0x0D40C
+#define HDMI_CORE_AUD_GP_INT			0x0D410
+#define HDMI_CORE_AUD_GP_POL			0x0D414
+#define HDMI_CORE_AUD_GP_MASK			0x0D418
+
+/* HDMI Main Controller */
+#define HDMI_CORE_MC_CLKDIS			0x10004
+#define HDMI_CORE_MC_SWRSTZREQ			0x10008
+#define HDMI_CORE_MC_FLOWCTRL			0x10010
+#define HDMI_CORE_MC_PHYRSTZ			0x10014
+#define HDMI_CORE_MC_LOCKONCLOCK		0x10018
+
+/* HDMI COLOR SPACE CONVERTER */
+#define HDMI_CORE_CSC_CFG			0x10400
+#define HDMI_CORE_CSC_SCALE			0x10404
+#define HDMI_CORE_CSC_COEF_A1_MSB		0x10408
+#define HDMI_CORE_CSC_COEF_A1_LSB		0x1040C
+#define HDMI_CORE_CSC_COEF_A2_MSB		0x10410
+#define HDMI_CORE_CSC_COEF_A2_LSB		0x10414
+#define HDMI_CORE_CSC_COEF_A3_MSB		0x10418
+#define HDMI_CORE_CSC_COEF_A3_LSB		0x1041C
+#define HDMI_CORE_CSC_COEF_A4_MSB		0x10420
+#define HDMI_CORE_CSC_COEF_A4_LSB		0x10424
+#define HDMI_CORE_CSC_COEF_B1_MSB		0x10428
+#define HDMI_CORE_CSC_COEF_B1_LSB		0x1042C
+#define HDMI_CORE_CSC_COEF_B2_MSB		0x10430
+#define HDMI_CORE_CSC_COEF_B2_LSB		0x10434
+#define HDMI_CORE_CSC_COEF_B3_MSB		0x10438
+#define HDMI_CORE_CSC_COEF_B3_LSB		0x1043C
+#define HDMI_CORE_CSC_COEF_B4_MSB		0x10440
+#define HDMI_CORE_CSC_COEF_B4_LSB		0x10444
+#define HDMI_CORE_CSC_COEF_C1_MSB		0x10448
+#define HDMI_CORE_CSC_COEF_C1_LSB		0x1044C
+#define HDMI_CORE_CSC_COEF_C2_MSB		0x10450
+#define HDMI_CORE_CSC_COEF_C2_LSB		0x10454
+#define HDMI_CORE_CSC_COEF_C3_MSB		0x10458
+#define HDMI_CORE_CSC_COEF_C3_LSB		0x1045C
+#define HDMI_CORE_CSC_COEF_C4_MSB		0x10460
+#define HDMI_CORE_CSC_COEF_C4_LSB		0x10464
+
+/* HDMI HDCP */
+#define HDMI_CORE_HDCP_MASK			0x14020
+
+/* HDMI CEC */
+#define HDMI_CORE_CEC_MASK			0x17408
+
+/* HDMI I2C Master */
+#define HDMI_CORE_I2CM_SLAVE			0x157C8
+#define HDMI_CORE_I2CM_ADDRESS			0x157CC
+#define HDMI_CORE_I2CM_DATAO			0x157D0
+#define HDMI_CORE_I2CM_DATAI			0X157D4
+#define HDMI_CORE_I2CM_OPERATION		0x157D8
+#define HDMI_CORE_I2CM_INT			0x157DC
+#define HDMI_CORE_I2CM_CTLINT			0x157E0
+#define HDMI_CORE_I2CM_DIV			0x157E4
+#define HDMI_CORE_I2CM_SEGADDR			0x157E8
+#define HDMI_CORE_I2CM_SOFTRSTZ			0x157EC
+#define HDMI_CORE_I2CM_SEGPTR			0x157F0
+#define HDMI_CORE_I2CM_SS_SCL_HCNT_1_ADDR	0x157F4
+#define HDMI_CORE_I2CM_SS_SCL_HCNT_0_ADDR	0x157F8
+#define HDMI_CORE_I2CM_SS_SCL_LCNT_1_ADDR	0x157FC
+#define HDMI_CORE_I2CM_SS_SCL_LCNT_0_ADDR	0x15800
+#define HDMI_CORE_I2CM_FS_SCL_HCNT_1_ADDR	0x15804
+#define HDMI_CORE_I2CM_FS_SCL_HCNT_0_ADDR	0x15808
+#define HDMI_CORE_I2CM_FS_SCL_LCNT_1_ADDR	0x1580C
+#define HDMI_CORE_I2CM_FS_SCL_LCNT_0_ADDR	0x15810
+#define HDMI_CORE_I2CM_SDA_HOLD_ADDR		0x15814
+
+enum hdmi_core_packet_mode {
+	HDMI_PACKETMODERESERVEDVALUE = 0,
+	HDMI_PACKETMODE24BITPERPIXEL = 4,
+	HDMI_PACKETMODE30BITPERPIXEL = 5,
+	HDMI_PACKETMODE36BITPERPIXEL = 6,
+	HDMI_PACKETMODE48BITPERPIXEL = 7,
+};
+
+struct hdmi_core_vid_config {
+	struct hdmi_config v_fc_config;
+	enum hdmi_core_packet_mode packet_mode;
+	int data_enable_pol;
+	int vblank_osc;
+	int hblank;
+	int vblank;
+};
+
+struct csc_table {
+	u16 a1, a2, a3, a4;
+	u16 b1, b2, b3, b4;
+	u16 c1, c2, c3, c4;
+};
+
+int hdmi5_read_edid(struct hdmi_core_data *core, u8 *edid, int len);
+void hdmi5_core_dump(struct hdmi_core_data *core, struct seq_file *s);
+int hdmi5_core_handle_irqs(struct hdmi_core_data *core);
+void hdmi5_configure(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
+			struct hdmi_config *cfg);
+int hdmi5_core_init(struct platform_device *pdev, struct hdmi_core_data *core);
+
+#if defined(CONFIG_OMAP5_DSS_HDMI_AUDIO)
+int hdmi5_audio_config(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
+			struct omap_dss_audio *audio, u32 pclk);
+#endif
+#endif
-- 
1.8.1.2


^ permalink raw reply related

* [PATCH 3/6] omapdss: hdmi phy: Incorporate changes for OMAP5/DRA7x
From: Archit Taneja @ 2013-10-17 11:38 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1382009202-18984-1-git-send-email-archit@ti.com>

OMAP5/DRA7x HDMI PHY has some differences compared to OMAP4 HDMI PHY.

Create a features struct which help the diagram configure the PHY based
on what SoC it is.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/hdmi.h     |  1 +
 drivers/video/omap2/dss/hdmi_phy.c | 94 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.h b/drivers/video/omap2/dss/hdmi.h
index b410b14..8fa2121 100644
--- a/drivers/video/omap2/dss/hdmi.h
+++ b/drivers/video/omap2/dss/hdmi.h
@@ -80,6 +80,7 @@
 #define HDMI_TXPHY_DIGITAL_CTRL			0x4
 #define HDMI_TXPHY_POWER_CTRL			0x8
 #define HDMI_TXPHY_PAD_CFG_CTRL			0xC
+#define HDMI_TXPHY_BIST_CONTROL			0x1C
 
 enum hdmi_pll_pwr {
 	HDMI_PLLPWRCMD_ALLOFF = 0,
diff --git a/drivers/video/omap2/dss/hdmi_phy.c b/drivers/video/omap2/dss/hdmi_phy.c
index 45acb99..4d9debb 100644
--- a/drivers/video/omap2/dss/hdmi_phy.c
+++ b/drivers/video/omap2/dss/hdmi_phy.c
@@ -12,11 +12,22 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/slab.h>
 #include <video/omapdss.h>
 
 #include "dss.h"
 #include "hdmi.h"
 
+struct hdmi_phy_features {
+	bool bist_ctrl;
+	bool calc_freqout;
+	bool ldo_voltage;
+	unsigned long dcofreq_min;	/* in KHz */
+	unsigned long max_phy;		/* in KHz */
+};
+
+static const struct hdmi_phy_features *phy_feat;
+
 void hdmi_phy_dump(struct hdmi_phy_data *phy, struct seq_file *s)
 {
 #define DUMPPHY(r) seq_printf(s, "%-35s %08x\n", #r,\
@@ -26,6 +37,8 @@ void hdmi_phy_dump(struct hdmi_phy_data *phy, struct seq_file *s)
 	DUMPPHY(HDMI_TXPHY_DIGITAL_CTRL);
 	DUMPPHY(HDMI_TXPHY_POWER_CTRL);
 	DUMPPHY(HDMI_TXPHY_PAD_CFG_CTRL);
+	if (phy_feat->bist_ctrl)
+		DUMPPHY(HDMI_TXPHY_BIST_CONTROL);
 }
 
 static irqreturn_t hdmi_irq_handler(int irq, void *data)
@@ -64,12 +77,20 @@ int hdmi_phy_enable(struct hdmi_phy_data *phy, struct hdmi_wp_data *wp,
 {
 	u16 r = 0;
 	u32 irqstatus;
+	u8 freqout;
 
 	hdmi_wp_clear_irqenable(wp, 0xffffffff);
 
 	irqstatus = hdmi_wp_get_irqstatus(wp);
 	hdmi_wp_set_irqstatus(wp, irqstatus);
 
+	/*
+	 * In OMAP5+, the HFBITCLK must be divided by 2 before issuing the
+	 * HDMI_PHYPWRCMD_LDOON command.
+	*/
+	if (phy_feat->bist_ctrl)
+		REG_FLD_MOD(phy->base, HDMI_TXPHY_BIST_CONTROL, 1, 11, 11);
+
 	r = hdmi_wp_set_phy_pwr(wp, HDMI_PHYPWRCMD_LDOON);
 	if (r)
 		return r;
@@ -80,17 +101,33 @@ int hdmi_phy_enable(struct hdmi_phy_data *phy, struct hdmi_wp_data *wp,
 	 */
 	hdmi_read_reg(phy->base, HDMI_TXPHY_TX_CTRL);
 
+	if (phy_feat->calc_freqout) {
+		/* DCOCLK/10 is pixel clock, compare pclk with DCOCLK_MIN/10 */
+		u32 dco_min = phy_feat->dcofreq_min / 10;
+		u32 pclk = cfg->timings.pixel_clock;
+
+		if (pclk < dco_min)
+			freqout = 0;
+		else if ((pclk >= dco_min) && (pclk < phy_feat->max_phy))
+			freqout = 1;
+		else
+			freqout = 2;
+	} else {
+		freqout = 1;
+	}
+
 	/*
 	 * Write to phy address 0 to configure the clock
 	 * use HFBITCLK write HDMI_TXPHY_TX_CONTROL_FREQOUT field
 	 */
-	REG_FLD_MOD(phy->base, HDMI_TXPHY_TX_CTRL, 0x1, 31, 30);
+	REG_FLD_MOD(phy->base, HDMI_TXPHY_TX_CTRL, freqout, 31, 30);
 
 	/* Write to phy address 1 to start HDMI line (TXVALID and TMDSCLKEN) */
 	hdmi_write_reg(phy->base, HDMI_TXPHY_DIGITAL_CTRL, 0xF0000000);
 
 	/* Setup max LDO voltage */
-	REG_FLD_MOD(phy->base, HDMI_TXPHY_POWER_CTRL, 0xB, 3, 0);
+	if (phy_feat->ldo_voltage)
+		REG_FLD_MOD(phy->base, HDMI_TXPHY_POWER_CTRL, 0xB, 3, 0);
 
 	/* Write to phy address 3 to change the polarity control */
 	REG_FLD_MOD(phy->base, HDMI_TXPHY_PAD_CFG_CTRL, 0x1, 27, 27);
@@ -119,11 +156,64 @@ void hdmi_phy_disable(struct hdmi_phy_data *phy, struct hdmi_wp_data *wp)
 #define PHY_OFFSET	0x300
 #define PHY_SIZE	0x100
 
+static const struct hdmi_phy_features omap44xx_phy_feats = {
+	.bist_ctrl	=	false,
+	.calc_freqout	=	false,
+	.ldo_voltage	=	true,
+	.dcofreq_min	=	500000,
+	.max_phy	=	185675,
+};
+
+static const struct hdmi_phy_features omap54xx_phy_feats = {
+	.bist_ctrl	=	true,
+	.calc_freqout	=	true,
+	.ldo_voltage	=	false,
+	.dcofreq_min	=	750000,
+	.max_phy	=	186000,
+};
+
+static int hdmi_phy_init_features(struct platform_device *pdev)
+{
+	struct hdmi_phy_features *dst;
+	const struct hdmi_phy_features *src;
+
+	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
+	if (!dst) {
+		dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n");
+		return -ENOMEM;
+	}
+
+	switch (omapdss_get_version()) {
+	case OMAPDSS_VER_OMAP4430_ES1:
+	case OMAPDSS_VER_OMAP4430_ES2:
+	case OMAPDSS_VER_OMAP4:
+		src = &omap44xx_phy_feats;
+		break;
+
+	case OMAPDSS_VER_OMAP5:
+		src = &omap54xx_phy_feats;
+		break;
+
+	default:
+		return -ENODEV;
+	}
+
+	memcpy(dst, src, sizeof(*dst));
+	phy_feat = dst;
+
+	return 0;
+}
+
 int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy)
 {
+	int r;
 	struct resource *res;
 	struct resource temp_res;
 
+	r = hdmi_phy_init_features(pdev);
+	if (r)
+		return r;
+
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hdmi_txphy");
 	if (!res) {
 		DSSDBG("can't get PHY mem resource by name\n");
-- 
1.8.1.2


^ permalink raw reply related

* [PATCH 4/6] omapdss: hdmi pll: Incorporate changes for OMAP5/DRA7x
From: Archit Taneja @ 2013-10-17 11:38 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1382009202-18984-1-git-send-email-archit@ti.com>

Add a features struct to differentiate between the HDMI PLLs on OMAP4 and
OMAP5/DRA7x.

The OMAP5/DRA7x PLL are more sensitive compared to the PLL on OMAP4 when it
comes to locking. We need to ensure that the DCO freq calculated isn't too low
for lower pixel clocks. If not followed, the PLL doesn't lock for lower
frequencies.

Modify the PLL computation slightly to ensure the HDMI PLL locks for lower
frequencies. This will be replaced by a more complex computation which makes
sure all the PLL constraints are met.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/hdmi_pll.c | 74 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 71 insertions(+), 3 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi_pll.c b/drivers/video/omap2/dss/hdmi_pll.c
index d3e6e78..e1c55c4 100644
--- a/drivers/video/omap2/dss/hdmi_pll.c
+++ b/drivers/video/omap2/dss/hdmi_pll.c
@@ -21,6 +21,13 @@
 #define HDMI_DEFAULT_REGN 16
 #define HDMI_DEFAULT_REGM2 1
 
+struct hdmi_pll_features {
+	bool sys_reset;
+	bool bound_dcofreq;
+};
+
+static const struct hdmi_pll_features *pll_feat;
+
 void hdmi_pll_dump(struct hdmi_pll_data *pll, struct seq_file *s)
 {
 #define DUMPPLL(r) seq_printf(s, "%-35s %08x\n", #r,\
@@ -55,7 +62,17 @@ void hdmi_pll_compute(struct hdmi_pll_data *pll, unsigned long clkin, int phy)
 
 	refclk = clkin / pi->regn;
 
-	pi->regm2 = HDMI_DEFAULT_REGM2;
+	/*
+	 * HACK: the HDMI PLL on omap5/dra7x doesn't lock if the required
+	 * DCOFREQ is lesser than the minimum value of the DCO's lower frequency
+	 * range. For now, we just make sure that low pixel clock rates also
+	 * generate a high enough DCOFREQ by setting the M2 post divider to a
+	 * higher value
+	 */
+	if (pll_feat->bound_dcofreq && phy <= 65000)
+		pi->regm2 = 3;
+	else
+		pi->regm2 = HDMI_DEFAULT_REGM2;
 
 	/*
 	 * multiplier is pixel_clk/ref_clk
@@ -151,8 +168,12 @@ static int hdmi_pll_config(struct hdmi_pll_data *pll)
 
 static int hdmi_pll_reset(struct hdmi_pll_data *pll)
 {
-	/* SYSRESET  controlled by power FSM */
-	REG_FLD_MOD(pll->base, PLLCTRL_PLL_CONTROL, 0x0, 3, 3);
+	/*
+	 * SYSRESET controlled by power FSM.
+	 * SYSRESETN needs to be set for omap5/dra7x, but cleared for omap4
+	 * HDMI PLL
+	 */
+	REG_FLD_MOD(pll->base, PLLCTRL_PLL_CONTROL, pll_feat->sys_reset, 3, 3);
 
 	/* READ 0x0 reset is in progress */
 	if (hdmi_wait_for_bit_change(pll->base, PLLCTRL_PLL_STATUS, 0, 0, 1)
@@ -195,11 +216,58 @@ void hdmi_pll_disable(struct hdmi_pll_data *pll, struct hdmi_wp_data *wp)
 #define PLL_OFFSET	0x200
 #define PLL_SIZE	0x100
 
+static const struct hdmi_pll_features omap44xx_pll_feats = {
+	.sys_reset		=	false,
+	.bound_dcofreq		=	false,
+};
+
+static const struct hdmi_pll_features omap54xx_pll_feats = {
+	.sys_reset		=	true,
+	.bound_dcofreq		=	true,
+};
+
+static int __init hdmi_pll_init_features(struct platform_device *pdev)
+{
+	struct hdmi_pll_features *dst;
+	const struct hdmi_pll_features *src;
+
+	dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL);
+	if (!dst) {
+		dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n");
+		return -ENOMEM;
+	}
+
+	switch (omapdss_get_version()) {
+	case OMAPDSS_VER_OMAP4430_ES1:
+	case OMAPDSS_VER_OMAP4430_ES2:
+	case OMAPDSS_VER_OMAP4:
+		src = &omap44xx_pll_feats;
+		break;
+
+	case OMAPDSS_VER_OMAP5:
+		src = &omap54xx_pll_feats;
+		break;
+
+	default:
+		return -ENODEV;
+	}
+
+	memcpy(dst, src, sizeof(*dst));
+	pll_feat = dst;
+
+	return 0;
+}
+
 int hdmi_pll_init(struct platform_device *pdev, struct hdmi_pll_data *pll)
 {
+	int r;
 	struct resource *res;
 	struct resource temp_res;
 
+	r = hdmi_pll_init_features(pdev);
+	if (r)
+		return r;
+
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hdmi_pllctrl");
 	if (!res) {
 		DSSDBG("can't get PLL mem resource by name\n");
-- 
1.8.1.2


^ permalink raw reply related

* [PATCH 5/6] omapdss: hdmi: add OMAP5/DRA7x hdmi driver
From: Archit Taneja @ 2013-10-17 11:38 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1382009202-18984-1-git-send-email-archit@ti.com>

The HDMI IP on OMAP5/DRA7x has a different core sub block compared to the one
one OMAP4. The rest of the IPs are the same. Add a hdmi5 platform driver which
calls hdmi5_core.c apis. The rest of the driver uses the same api's for wrapper,
pll and phy, and the common helper funcs for timings.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/Kconfig   |  10 +
 drivers/video/omap2/dss/Makefile  |   2 +
 drivers/video/omap2/dss/core.c    |   6 +
 drivers/video/omap2/dss/dss.h     |   3 +
 drivers/video/omap2/dss/hdmi.h    |   2 +-
 drivers/video/omap2/dss/hdmi5.c   | 699 ++++++++++++++++++++++++++++++++++++++
 drivers/video/omap2/dss/hdmi_wp.c |   2 +-
 7 files changed, 722 insertions(+), 2 deletions(-)
 create mode 100644 drivers/video/omap2/dss/hdmi5.c

diff --git a/drivers/video/omap2/dss/Kconfig b/drivers/video/omap2/dss/Kconfig
index dde4281..a00932f 100644
--- a/drivers/video/omap2/dss/Kconfig
+++ b/drivers/video/omap2/dss/Kconfig
@@ -69,6 +69,16 @@ config OMAP4_DSS_HDMI
 config OMAP4_DSS_HDMI_AUDIO
 	bool
 
+config OMAP5_DSS_HDMI
+	bool "OMAP5 HDMI support"
+        default y
+	help
+	  HDMI Interface for OMAP5 and SoCs containing the same HDMI core IP.
+
+config OMAP5_DSS_HDMI_AUDIO
+	depends on OMAP5_DSS_HDMI
+	bool
+
 config OMAP2_DSS_SDI
 	bool "SDI support"
         default n
diff --git a/drivers/video/omap2/dss/Makefile b/drivers/video/omap2/dss/Makefile
index d3aa91b..89d83be 100644
--- a/drivers/video/omap2/dss/Makefile
+++ b/drivers/video/omap2/dss/Makefile
@@ -12,4 +12,6 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
 omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
 omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi_common.o hdmi_wp.o hdmi_pll.o \
 	hdmi_phy.o hdmi4_core.o
+omapdss-$(CONFIG_OMAP5_DSS_HDMI) += hdmi5.o hdmi_common.o hdmi_wp.o hdmi_pll.o \
+	hdmi_phy.o hdmi5_core.o
 ccflags-$(CONFIG_OMAP2_DSS_DEBUG) += -DDEBUG
diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index ffa45c8..6b74f73 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -268,6 +268,9 @@ static int (*dss_output_drv_reg_funcs[])(void) __initdata = {
 #ifdef CONFIG_OMAP4_DSS_HDMI
 	hdmi4_init_platform_driver,
 #endif
+#ifdef CONFIG_OMAP5_DSS_HDMI
+	hdmi5_init_platform_driver,
+#endif
 };
 
 static void (*dss_output_drv_unreg_funcs[])(void) __exitdata = {
@@ -289,6 +292,9 @@ static void (*dss_output_drv_unreg_funcs[])(void) __exitdata = {
 #ifdef CONFIG_OMAP4_DSS_HDMI
 	hdmi4_uninit_platform_driver,
 #endif
+#ifdef CONFIG_OMAP5_DSS_HDMI
+	hdmi5_uninit_platform_driver,
+#endif
 };
 
 static bool dss_output_drv_loaded[ARRAY_SIZE(dss_output_drv_reg_funcs)];
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index f777962..bb8a39c 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -430,6 +430,9 @@ void venc_uninit_platform_driver(void) __exit;
 int hdmi4_init_platform_driver(void) __init;
 void hdmi4_uninit_platform_driver(void) __exit;
 
+int hdmi5_init_platform_driver(void) __init;
+void hdmi5_uninit_platform_driver(void) __exit;
+
 /* RFBI */
 int rfbi_init_platform_driver(void) __init;
 void rfbi_uninit_platform_driver(void) __exit;
diff --git a/drivers/video/omap2/dss/hdmi.h b/drivers/video/omap2/dss/hdmi.h
index 8fa2121..2b2db2d 100644
--- a/drivers/video/omap2/dss/hdmi.h
+++ b/drivers/video/omap2/dss/hdmi.h
@@ -429,7 +429,7 @@ const struct hdmi_config *hdmi_default_timing(void);
 const struct hdmi_config *hdmi_get_timings(int mode, int code);
 struct hdmi_cm hdmi_get_code(struct omap_video_timings *timing);
 
-#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO) || defined(CONFIG_OMAP5_DSS_HDMI_AUDIO)
 int hdmi_compute_acr(u32 pclk, u32 sample_freq, u32 *n, u32 *cts);
 int hdmi_wp_audio_enable(struct hdmi_wp_data *wp, bool enable);
 int hdmi_wp_audio_core_req_enable(struct hdmi_wp_data *wp, bool enable);
diff --git a/drivers/video/omap2/dss/hdmi5.c b/drivers/video/omap2/dss/hdmi5.c
new file mode 100644
index 0000000..5a1cfd6
--- /dev/null
+++ b/drivers/video/omap2/dss/hdmi5.c
@@ -0,0 +1,699 @@
+/*
+ * hdmi.c
+ *
+ * HDMI interface DSS driver setting for TI's OMAP4 family of processor.
+ * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com/
+ * Authors: Yong Zhi
+ *	Mythri pk <mythripk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define DSS_SUBSYS_NAME "HDMI"
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+#include <linux/string.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/regulator/consumer.h>
+#include <video/omapdss.h>
+
+#include "hdmi5_core.h"
+#include "dss.h"
+#include "dss_features.h"
+
+static struct {
+	struct mutex lock;
+	struct platform_device *pdev;
+
+	struct hdmi_wp_data	wp;
+	struct hdmi_pll_data	pll;
+	struct hdmi_phy_data	phy;
+	struct hdmi_core_data	core;
+
+	struct hdmi_config cfg;
+
+	struct clk *sys_clk;
+	struct regulator *vdda_hdmi_dac_reg;
+
+	bool core_enabled;
+
+	struct omap_dss_device output;
+} hdmi;
+
+static int hdmi_runtime_get(void)
+{
+	int r;
+
+	DSSDBG("hdmi_runtime_get\n");
+
+	r = pm_runtime_get_sync(&hdmi.pdev->dev);
+	WARN_ON(r < 0);
+	if (r < 0)
+		return r;
+
+	return 0;
+}
+
+static void hdmi_runtime_put(void)
+{
+	int r;
+
+	DSSDBG("hdmi_runtime_put\n");
+
+	r = pm_runtime_put_sync(&hdmi.pdev->dev);
+	WARN_ON(r < 0 && r != -ENOSYS);
+}
+
+static int hdmi_init_regulator(void)
+{
+	struct regulator *reg;
+
+	if (hdmi.vdda_hdmi_dac_reg != NULL)
+		return 0;
+
+	reg = devm_regulator_get(&hdmi.pdev->dev, "vdda_hdmi_dac");
+	if (IS_ERR(reg)) {
+		DSSERR("can't get VDDA_HDMI_DAC regulator\n");
+		return PTR_ERR(reg);
+	}
+
+	hdmi.vdda_hdmi_dac_reg = reg;
+
+	return 0;
+}
+
+static int hdmi_power_on_core(struct omap_dss_device *dssdev)
+{
+	int r;
+
+	r = regulator_enable(hdmi.vdda_hdmi_dac_reg);
+	if (r)
+		return r;
+
+	r = hdmi_runtime_get();
+	if (r)
+		goto err_runtime_get;
+
+	/* Make selection of HDMI in DSS */
+	dss_select_hdmi_venc_clk_source(DSS_HDMI_M_PCLK);
+
+	hdmi.core_enabled = true;
+
+	return 0;
+
+err_runtime_get:
+	regulator_disable(hdmi.vdda_hdmi_dac_reg);
+
+	return r;
+}
+
+static void hdmi_power_off_core(struct omap_dss_device *dssdev)
+{
+	hdmi.core_enabled = false;
+
+	hdmi_runtime_put();
+	regulator_disable(hdmi.vdda_hdmi_dac_reg);
+}
+
+static int hdmi_power_on_full(struct omap_dss_device *dssdev)
+{
+	int r;
+	struct omap_video_timings *p;
+	struct omap_overlay_manager *mgr = hdmi.output.manager;
+	unsigned long phy;
+
+	r = hdmi_power_on_core(dssdev);
+	if (r)
+		return r;
+
+	dss_mgr_disable(mgr);
+
+	p = &hdmi.cfg.timings;
+
+	DSSDBG("hdmi_power_on x_res= %d y_res = %d\n", p->x_res, p->y_res);
+
+	phy = p->pixel_clock;
+
+	hdmi_pll_compute(&hdmi.pll, clk_get_rate(hdmi.sys_clk), phy);
+
+	hdmi_wp_video_stop(&hdmi.wp);
+
+	/* config the PLL and PHY hdmi_set_pll_pwrfirst */
+	r = hdmi_pll_enable(&hdmi.pll, &hdmi.wp);
+	if (r) {
+		DSSDBG("Failed to lock PLL\n");
+		goto err_pll_enable;
+	}
+
+	r = hdmi_phy_enable(&hdmi.phy, &hdmi.wp, &hdmi.cfg);
+	if (r) {
+		DSSDBG("Failed to start PHY\n");
+		goto err_phy_enable;
+	}
+
+	hdmi5_configure(&hdmi.core, &hdmi.wp, &hdmi.cfg);
+
+	/* bypass TV gamma table */
+	dispc_enable_gamma_table(0);
+
+	/* tv size */
+	dss_mgr_set_timings(mgr, p);
+
+	r = hdmi_wp_video_start(&hdmi.wp);
+	if (r)
+		goto err_vid_enable;
+
+	r = dss_mgr_enable(mgr);
+	if (r)
+		goto err_mgr_enable;
+
+	return 0;
+
+err_mgr_enable:
+	hdmi_wp_video_stop(&hdmi.wp);
+err_vid_enable:
+	hdmi_phy_disable(&hdmi.phy, &hdmi.wp);
+err_phy_enable:
+	hdmi_pll_disable(&hdmi.pll, &hdmi.wp);
+err_pll_enable:
+	hdmi_power_off_core(dssdev);
+	return -EIO;
+}
+
+static void hdmi_power_off_full(struct omap_dss_device *dssdev)
+{
+	struct omap_overlay_manager *mgr = hdmi.output.manager;
+
+	dss_mgr_disable(mgr);
+
+	hdmi_wp_video_stop(&hdmi.wp);
+	hdmi_phy_disable(&hdmi.phy, &hdmi.wp);
+	hdmi_pll_disable(&hdmi.pll, &hdmi.wp);
+
+	hdmi_power_off_core(dssdev);
+}
+
+static int hdmi_display_check_timing(struct omap_dss_device *dssdev,
+					struct omap_video_timings *timings)
+{
+	struct hdmi_cm cm;
+
+	cm = hdmi_get_code(timings);
+	if (cm.code = -1)
+		return -EINVAL;
+
+	return 0;
+
+}
+
+static void hdmi_display_set_timing(struct omap_dss_device *dssdev,
+		struct omap_video_timings *timings)
+{
+	struct hdmi_cm cm;
+	const struct hdmi_config *t;
+
+	mutex_lock(&hdmi.lock);
+
+	cm = hdmi_get_code(timings);
+	hdmi.cfg.cm = cm;
+
+	t = hdmi_get_timings(cm.mode, cm.code);
+	if (t != NULL) {
+		hdmi.cfg = *t;
+
+		dispc_set_tv_pclk(t->timings.pixel_clock * 1000);
+	}
+
+	mutex_unlock(&hdmi.lock);
+}
+
+static void hdmi_display_get_timings(struct omap_dss_device *dssdev,
+		struct omap_video_timings *timings)
+{
+	const struct hdmi_config *cfg;
+	struct hdmi_cm cm = hdmi.cfg.cm;
+
+	cfg = hdmi_get_timings(cm.mode, cm.code);
+	if (cfg = NULL)
+		cfg = hdmi_default_timing();
+
+	memcpy(timings, &cfg->timings, sizeof(cfg->timings));
+}
+
+static void hdmi_dump_regs(struct seq_file *s)
+{
+	mutex_lock(&hdmi.lock);
+
+	if (hdmi_runtime_get()) {
+		mutex_unlock(&hdmi.lock);
+		return;
+	}
+
+	hdmi_wp_dump(&hdmi.wp, s);
+	hdmi_pll_dump(&hdmi.pll, s);
+	hdmi_phy_dump(&hdmi.phy, s);
+	hdmi5_core_dump(&hdmi.core, s);
+
+	hdmi_runtime_put();
+	mutex_unlock(&hdmi.lock);
+}
+
+static int read_edid(u8 *buf, int len)
+{
+	int r;
+
+	mutex_lock(&hdmi.lock);
+
+	r = hdmi_runtime_get();
+	BUG_ON(r);
+
+	r = hdmi5_read_edid(&hdmi.core,  buf, len);
+
+	hdmi_runtime_put();
+	mutex_unlock(&hdmi.lock);
+
+	return r;
+}
+
+static int hdmi_display_enable(struct omap_dss_device *dssdev)
+{
+	struct omap_dss_device *out = &hdmi.output;
+	int r = 0;
+
+	DSSDBG("ENTER hdmi_display_enable\n");
+
+	mutex_lock(&hdmi.lock);
+
+	if (out = NULL || out->manager = NULL) {
+		DSSERR("failed to enable display: no output/manager\n");
+		r = -ENODEV;
+		goto err0;
+	}
+
+	r = hdmi_power_on_full(dssdev);
+	if (r) {
+		DSSERR("failed to power on device\n");
+		goto err0;
+	}
+
+	mutex_unlock(&hdmi.lock);
+	return 0;
+
+err0:
+	mutex_unlock(&hdmi.lock);
+	return r;
+}
+
+static void hdmi_display_disable(struct omap_dss_device *dssdev)
+{
+	DSSDBG("Enter hdmi_display_disable\n");
+
+	mutex_lock(&hdmi.lock);
+
+	hdmi_power_off_full(dssdev);
+
+	mutex_unlock(&hdmi.lock);
+}
+
+static int hdmi_core_enable(struct omap_dss_device *dssdev)
+{
+	int r = 0;
+
+	DSSDBG("ENTER omapdss_hdmi_core_enable\n");
+
+	mutex_lock(&hdmi.lock);
+
+	r = hdmi_power_on_core(dssdev);
+	if (r) {
+		DSSERR("failed to power on device\n");
+		goto err0;
+	}
+
+	mutex_unlock(&hdmi.lock);
+	return 0;
+
+err0:
+	mutex_unlock(&hdmi.lock);
+	return r;
+}
+
+static void hdmi_core_disable(struct omap_dss_device *dssdev)
+{
+	DSSDBG("Enter omapdss_hdmi_core_disable\n");
+
+	mutex_lock(&hdmi.lock);
+
+	hdmi_power_off_core(dssdev);
+
+	mutex_unlock(&hdmi.lock);
+}
+
+static int hdmi_get_clocks(struct platform_device *pdev)
+{
+	struct clk *clk;
+
+	clk = devm_clk_get(&pdev->dev, "sys_clk");
+	if (IS_ERR(clk)) {
+		DSSERR("can't get sys_clk\n");
+		return PTR_ERR(clk);
+	}
+
+	hdmi.sys_clk = clk;
+
+	return 0;
+}
+
+static int hdmi_connect(struct omap_dss_device *dssdev,
+		struct omap_dss_device *dst)
+{
+	struct omap_overlay_manager *mgr;
+	int r;
+
+	r = hdmi_init_regulator();
+	if (r)
+		return r;
+
+	mgr = omap_dss_get_overlay_manager(dssdev->dispc_channel);
+	if (!mgr)
+		return -ENODEV;
+
+	r = dss_mgr_connect(mgr, dssdev);
+	if (r)
+		return r;
+
+	r = omapdss_output_set_device(dssdev, dst);
+	if (r) {
+		DSSERR("failed to connect output to new device: %s\n",
+				dst->name);
+		dss_mgr_disconnect(mgr, dssdev);
+		return r;
+	}
+
+	return 0;
+}
+
+static void hdmi_disconnect(struct omap_dss_device *dssdev,
+		struct omap_dss_device *dst)
+{
+	WARN_ON(dst != dssdev->dst);
+
+	if (dst != dssdev->dst)
+		return;
+
+	omapdss_output_unset_device(dssdev);
+
+	if (dssdev->manager)
+		dss_mgr_disconnect(dssdev->manager, dssdev);
+}
+
+static int hdmi_read_edid(struct omap_dss_device *dssdev,
+		u8 *edid, int len)
+{
+	bool need_enable;
+	int r;
+
+	need_enable = hdmi.core_enabled = false;
+
+	if (need_enable) {
+		r = hdmi_core_enable(dssdev);
+		if (r)
+			return r;
+	}
+
+	r = read_edid(edid, len);
+
+	if (need_enable)
+		hdmi_core_disable(dssdev);
+
+	return r;
+}
+
+#if defined(CONFIG_OMAP5_DSS_HDMI_AUDIO)
+static int hdmi_audio_enable(struct omap_dss_device *dssdev)
+{
+	int r;
+
+	mutex_lock(&hdmi.lock);
+
+	if (!hdmi_mode_has_audio(hdmi.cfg.cm.mode)) {
+		r = -EPERM;
+		goto err;
+	}
+
+	r = hdmi_wp_audio_enable(&hdmi.wp, true);
+	if (r)
+		goto err;
+
+	mutex_unlock(&hdmi.lock);
+	return 0;
+
+err:
+	mutex_unlock(&hdmi.lock);
+	return r;
+}
+
+static void hdmi_audio_disable(struct omap_dss_device *dssdev)
+{
+	hdmi_wp_audio_enable(&hdmi.wp, false);
+}
+
+static int hdmi_audio_start(struct omap_dss_device *dssdev)
+{
+	return hdmi_wp_audio_core_req_enable(&hdmi.wp, true);
+}
+
+static void hdmi_audio_stop(struct omap_dss_device *dssdev)
+{
+	hdmi_wp_audio_core_req_enable(&hdmi.wp, false);
+}
+
+static bool hdmi_audio_supported(struct omap_dss_device *dssdev)
+{
+	bool r;
+
+	mutex_lock(&hdmi.lock);
+
+	r = hdmi_mode_has_audio(hdmi.cfg.cm.mode);
+
+	mutex_unlock(&hdmi.lock);
+	return r;
+}
+
+static int hdmi_audio_config(struct omap_dss_device *dssdev,
+		struct omap_dss_audio *audio)
+{
+	int r;
+	u32 pclk = hdmi.cfg.timings.pixel_clock;
+
+	mutex_lock(&hdmi.lock);
+
+	if (!hdmi_mode_has_audio(hdmi.cfg.cm.mode)) {
+		r = -EPERM;
+		goto err;
+	}
+
+	r = hdmi5_audio_config(&hdmi.core, &hdmi.wp, audio, pclk);
+	if (r)
+		goto err;
+
+	mutex_unlock(&hdmi.lock);
+	return 0;
+
+err:
+	mutex_unlock(&hdmi.lock);
+	return r;
+}
+#else
+static int hdmi_audio_enable(struct omap_dss_device *dssdev)
+{
+	return -EPERM;
+}
+
+static void hdmi_audio_disable(struct omap_dss_device *dssdev)
+{
+}
+
+static int hdmi_audio_start(struct omap_dss_device *dssdev)
+{
+	return -EPERM;
+}
+
+static void hdmi_audio_stop(struct omap_dss_device *dssdev)
+{
+}
+
+static bool hdmi_audio_supported(struct omap_dss_device *dssdev)
+{
+	return false;
+}
+
+static int hdmi_audio_config(struct omap_dss_device *dssdev,
+		struct omap_dss_audio *audio)
+{
+	return -EPERM;
+}
+#endif
+
+static const struct omapdss_hdmi_ops hdmi_ops = {
+	.connect		= hdmi_connect,
+	.disconnect		= hdmi_disconnect,
+
+	.enable			= hdmi_display_enable,
+	.disable		= hdmi_display_disable,
+
+	.check_timings		= hdmi_display_check_timing,
+	.set_timings		= hdmi_display_set_timing,
+	.get_timings		= hdmi_display_get_timings,
+
+	.read_edid		= hdmi_read_edid,
+
+	.audio_enable		= hdmi_audio_enable,
+	.audio_disable		= hdmi_audio_disable,
+	.audio_start		= hdmi_audio_start,
+	.audio_stop		= hdmi_audio_stop,
+	.audio_supported	= hdmi_audio_supported,
+	.audio_config		= hdmi_audio_config,
+};
+
+static void hdmi_init_output(struct platform_device *pdev)
+{
+	struct omap_dss_device *out = &hdmi.output;
+
+	out->dev = &pdev->dev;
+	out->id = OMAP_DSS_OUTPUT_HDMI;
+	out->output_type = OMAP_DISPLAY_TYPE_HDMI;
+	out->name = "hdmi.0";
+	out->dispc_channel = OMAP_DSS_CHANNEL_DIGIT;
+	out->ops.hdmi = &hdmi_ops;
+	out->owner = THIS_MODULE;
+
+	omapdss_register_output(out);
+}
+
+static void __exit hdmi_uninit_output(struct platform_device *pdev)
+{
+	struct omap_dss_device *out = &hdmi.output;
+
+	omapdss_unregister_output(out);
+}
+
+/* HDMI HW IP initialisation */
+static int omapdss_hdmihw_probe(struct platform_device *pdev)
+{
+	int r;
+
+	hdmi.pdev = pdev;
+
+	mutex_init(&hdmi.lock);
+
+	r = hdmi_wp_init(pdev, &hdmi.wp);
+	if (r)
+		return r;
+
+	r = hdmi_pll_init(pdev, &hdmi.pll);
+	if (r)
+		return r;
+
+	r = hdmi_phy_init(pdev, &hdmi.phy);
+	if (r)
+		return r;
+
+	r = hdmi5_core_init(pdev, &hdmi.core);
+	if (r)
+		return r;
+
+	r = hdmi_get_clocks(pdev);
+	if (r) {
+		DSSERR("can't get clocks\n");
+		return r;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+
+	hdmi_init_output(pdev);
+
+	dss_debugfs_create_file("hdmi", hdmi_dump_regs);
+
+	return 0;
+}
+
+static int __exit omapdss_hdmihw_remove(struct platform_device *pdev)
+{
+	hdmi_uninit_output(pdev);
+
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static int hdmi_runtime_suspend(struct device *dev)
+{
+	clk_disable_unprepare(hdmi.sys_clk);
+
+	dispc_runtime_put();
+
+	return 0;
+}
+
+static int hdmi_runtime_resume(struct device *dev)
+{
+	int r;
+
+	r = dispc_runtime_get();
+	if (r < 0)
+		return r;
+
+	clk_prepare_enable(hdmi.sys_clk);
+
+	return 0;
+}
+
+static const struct dev_pm_ops hdmi_pm_ops = {
+	.runtime_suspend = hdmi_runtime_suspend,
+	.runtime_resume = hdmi_runtime_resume,
+};
+
+static const struct of_device_id hdmi_of_match[] = {
+	{ .compatible = "ti,omap5-hdmi", },
+	{},
+};
+
+static struct platform_driver omapdss_hdmihw_driver = {
+	.probe		= omapdss_hdmihw_probe,
+	.remove         = __exit_p(omapdss_hdmihw_remove),
+	.driver         = {
+		.name   = "omapdss_hdmi5",
+		.owner  = THIS_MODULE,
+		.pm	= &hdmi_pm_ops,
+		.of_match_table = hdmi_of_match,
+	},
+};
+
+int __init hdmi5_init_platform_driver(void)
+{
+	return platform_driver_register(&omapdss_hdmihw_driver);
+}
+
+void __exit hdmi5_uninit_platform_driver(void)
+{
+	platform_driver_unregister(&omapdss_hdmihw_driver);
+}
diff --git a/drivers/video/omap2/dss/hdmi_wp.c b/drivers/video/omap2/dss/hdmi_wp.c
index 8151d89..a44f709 100644
--- a/drivers/video/omap2/dss/hdmi_wp.c
+++ b/drivers/video/omap2/dss/hdmi_wp.c
@@ -181,7 +181,7 @@ void hdmi_wp_init_vid_fmt_timings(struct hdmi_video_format *video_fmt,
 	timings->interlace = param->timings.interlace;
 }
 
-#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO) || defined(CONFIG_OMAP5_DSS_HDMI_AUDIO)
 void hdmi_wp_audio_config_format(struct hdmi_wp_data *wp,
 		struct hdmi_audio_format *aud_fmt)
 {
-- 
1.8.1.2


^ permalink raw reply related

* [PATCH 6/6] omapdss: hdmi4/hdmi5: set regulator voltage to required level
From: Archit Taneja @ 2013-10-17 11:38 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja, Somnath Mukherjee
In-Reply-To: <1382009202-18984-1-git-send-email-archit@ti.com>

It's not sufficient to request the regulator. We need to set the IO voltage as
well if the min and max voltage values of the regulator differ.

However, if the regulator on a platform doesn't permit the range we requested
for, or is a fixed voltage regulator, we just give a warning that we didn't get
voltage range we requested for. The recommended voltage range for vdda_hdmi as
per the HDMI PHY spec is 1.8V to 1.98V for OMAP4 and 1.5V to 1.8V for OMAP5.

Originally worked on by Somnath Mukherjee <somnath@ti.com>

Cc: Somnath Mukherjee <somnath@ti.com>
Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/hdmi4.c | 5 +++++
 drivers/video/omap2/dss/hdmi5.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/video/omap2/dss/hdmi4.c b/drivers/video/omap2/dss/hdmi4.c
index e140096..95638b8 100644
--- a/drivers/video/omap2/dss/hdmi4.c
+++ b/drivers/video/omap2/dss/hdmi4.c
@@ -83,6 +83,7 @@ static void hdmi_runtime_put(void)
 
 static int hdmi_init_regulator(void)
 {
+	int r;
 	struct regulator *reg;
 
 	if (hdmi.vdda_hdmi_dac_reg != NULL)
@@ -99,6 +100,10 @@ static int hdmi_init_regulator(void)
 		return PTR_ERR(reg);
 	}
 
+	r = regulator_set_voltage(reg, 1800000, 1980000);
+	if (r)
+		DSSWARN("can't set regulator voltage");
+
 	hdmi.vdda_hdmi_dac_reg = reg;
 
 	return 0;
diff --git a/drivers/video/omap2/dss/hdmi5.c b/drivers/video/omap2/dss/hdmi5.c
index 5a1cfd6..aa91f4e 100644
--- a/drivers/video/omap2/dss/hdmi5.c
+++ b/drivers/video/omap2/dss/hdmi5.c
@@ -85,6 +85,7 @@ static void hdmi_runtime_put(void)
 
 static int hdmi_init_regulator(void)
 {
+	int r;
 	struct regulator *reg;
 
 	if (hdmi.vdda_hdmi_dac_reg != NULL)
@@ -96,6 +97,10 @@ static int hdmi_init_regulator(void)
 		return PTR_ERR(reg);
 	}
 
+	r = regulator_set_voltage(reg, 1500000, 1800000);
+	if (r)
+		DSSWARN("can't set regulator voltage");
+
 	hdmi.vdda_hdmi_dac_reg = reg;
 
 	return 0;
-- 
1.8.1.2


^ 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