Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] video: amba-clcd: Make CLCD driver available on more platforms
From: Geert Uytterhoeven @ 2013-12-12 17:48 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1386868390-12231-1-git-send-email-broonie@kernel.org>

On Thu, Dec 12, 2013 at 6:13 PM, Mark Brown <broonie@kernel.org> wrote:
> The CLCD driver is used on ARM reference models for ARMv8 so add ARM64
> to the list of dependencies. The driver also has no build time dependencies
> on ARM (stubs are provided for ARM-specific DMA functions in the code) so
> make it available with COMPILE_TEST in order to maximise build coverage.
>
> Signed-off-by: Mark Brown <broonie@linaro.org>
> ---
>  drivers/video/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 4f2e1b35eb38..e6c7fb1a389b 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -312,7 +312,8 @@ config FB_PM2_FIFO_DISCONNECT
>
>  config FB_ARMCLCD
>         tristate "ARM PrimeCell PL110 support"
> -       depends on FB && ARM && ARM_AMBA
> +       depends on ARM || ARM64 || COMPILE_TEST
> +       depends on FB && ARM_AMBA
>         select FB_CFB_FILLRECT
>         select FB_CFB_COPYAREA
>         select FB_CFB_IMAGEBLIT

Currently ARM_AMBA exists on arm/arm64 only.
Is there a patch in the pipeline to change that? Without such a change,
this patch doesn't make any difference.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Sebastian Reichel @ 2013-12-12 17:31 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Laurent Pinchart, linux-omap, linux-fbdev, devicetree,
	Archit Taneja, Darren Etheridge, Tony Lindgren
In-Reply-To: <52A9C5D5.80801@ti.com>

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

On Thu, Dec 12, 2013 at 04:19:01PM +0200, Tomi Valkeinen wrote:
> On 2013-12-12 16:15, Laurent Pinchart wrote:
> 
> > As you mentioned in your previous e-mail, if the labels are used by omapfb 
> > only, I won't strongly push to keep them. I wonder, however, when using 
> > DRM/KMS, where do the connector labels that are displayed by xrandr for 
> > instance come from ?
> 
> drivers/gpu/drm/drm_crtc.c has lists, with names like "DVI-D", "HDMI-A",
> for connectors and encoders. Maybe from those.

The xrandr names are generated from the list Tomi mentioned.
=> drm_connector_enum_list

This requires a mapping from omapdss types to DRM types, which is
done in drivers/gpu/drm/omapdrm/omap_drv.c:get_connector_type().

Currently only HDMI and DVI are handled properly.

-- Sebastian

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

^ permalink raw reply

* [PATCH] video: amba-clcd: Make CLCD driver available on more platforms
From: Mark Brown @ 2013-12-12 17:13 UTC (permalink / raw)
  To: linux-fbdev

From: Mark Brown <broonie@linaro.org>

The CLCD driver is used on ARM reference models for ARMv8 so add ARM64
to the list of dependencies. The driver also has no build time dependencies
on ARM (stubs are provided for ARM-specific DMA functions in the code) so
make it available with COMPILE_TEST in order to maximise build coverage.

Signed-off-by: Mark Brown <broonie@linaro.org>
---
 drivers/video/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4f2e1b35eb38..e6c7fb1a389b 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -312,7 +312,8 @@ config FB_PM2_FIFO_DISCONNECT
 
 config FB_ARMCLCD
 	tristate "ARM PrimeCell PL110 support"
-	depends on FB && ARM && ARM_AMBA
+	depends on ARM || ARM64 || COMPILE_TEST
+	depends on FB && ARM_AMBA
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
-- 
1.8.5.1


^ permalink raw reply related

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Tomi Valkeinen @ 2013-12-12 14:19 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Sebastian Reichel, linux-omap, linux-fbdev, devicetree,
	Archit Taneja, Darren Etheridge, Tony Lindgren
In-Reply-To: <1793565.XC2oCqQxA8@avalon>

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

On 2013-12-12 16:15, Laurent Pinchart wrote:

> As you mentioned in your previous e-mail, if the labels are used by omapfb 
> only, I won't strongly push to keep them. I wonder, however, when using 
> DRM/KMS, where do the connector labels that are displayed by xrandr for 
> instance come from ?

drivers/gpu/drm/drm_crtc.c has lists, with names like "DVI-D", "HDMI-A",
for connectors and encoders. Maybe from those.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Laurent Pinchart @ 2013-12-12 14:15 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Sebastian Reichel, linux-omap, linux-fbdev, devicetree,
	Archit Taneja, Darren Etheridge, Tony Lindgren
In-Reply-To: <52A9C470.2030102@ti.com>

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

Hi Tomi,

On Thursday 12 December 2013 16:13:04 Tomi Valkeinen wrote:
> On 2013-12-12 12:05, Sebastian Reichel wrote:
> > On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote:
> >>> A label property is still an option.
> >> 
> >> Hmm, what do you mean? Label as in:
> >> 
> >> foo : node {
> >> };
> >> 
> >> Isn't that 'foo' label only visible in DT itself, as a shortcut?
> > 
> > Some driver use a "label" property like this:
> > 
> > foo : node {
> > 
> >     label = "lcd";
> >     
> >     ...
> > 
> > };
> > 
> > See for example
> > 
> > Documentation/devicetree/bindings/leds/common.txt
> > Documentation/devicetree/bindings/mtd/partition.txt
> 
> Ah, I see. That kind of label was actually the first thing I did when
> starting to work on DSS DT. But I removed it, as it didn't describe the
> hardware and I didn't see others using anything similar.
> 
> But I guess one could argue it does describe hardware, not in electrical
> level but in conceptual level.
> 
> The question is, do we need labeling for displays? For backward
> compatibility omapdss would need it, but in general? I'm quite content
> with having just display0, display1 etc. Using the alias node, those can
> be fixed and display0 is always the same display.

As you mentioned in your previous e-mail, if the labels are used by omapfb 
only, I won't strongly push to keep them. I wonder, however, when using 
DRM/KMS, where do the connector labels that are displayed by xrandr for 
instance come from ?

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Tomi Valkeinen @ 2013-12-12 14:13 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Laurent Pinchart, linux-omap, linux-fbdev, devicetree,
	Archit Taneja, Darren Etheridge, Tony Lindgren
In-Reply-To: <20131212100527.GA860@earth.universe>

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

On 2013-12-12 12:05, Sebastian Reichel wrote:
> On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote:
>>> A label property is still an option.
>>
>> Hmm, what do you mean? Label as in:
>>
>> foo : node {
>> };
>>
>> Isn't that 'foo' label only visible in DT itself, as a shortcut?
> 
> Some driver use a "label" property like this:
> 
> foo : node {
>     label = "lcd";
> 
>     ...
> };
> 
> See for example
> 
> Documentation/devicetree/bindings/leds/common.txt
> Documentation/devicetree/bindings/mtd/partition.txt

Ah, I see. That kind of label was actually the first thing I did when
starting to work on DSS DT. But I removed it, as it didn't describe the
hardware and I didn't see others using anything similar.

But I guess one could argue it does describe hardware, not in electrical
level but in conceptual level.

The question is, do we need labeling for displays? For backward
compatibility omapdss would need it, but in general? I'm quite content
with having just display0, display1 etc. Using the alias node, those can
be fixed and display0 is always the same display.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Laurent Pinchart @ 2013-12-12 13:22 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Tomi Valkeinen, linux-omap, linux-fbdev, devicetree,
	Archit Taneja, Darren Etheridge, Tony Lindgren
In-Reply-To: <20131212100527.GA860@earth.universe>

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

On Thursday 12 December 2013 11:05:28 Sebastian Reichel wrote:
> On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote:
> > > A label property is still an option.
> > 
> > Hmm, what do you mean? Label as in:
> > 
> > foo : node {
> > };
> > 
> > Isn't that 'foo' label only visible in DT itself, as a shortcut?
> 
> Some driver use a "label" property like this:
> 
> foo : node {
>     label = "lcd";
> 
>     ...
> };

Yes, that's what I meant.

> See for example
> 
> Documentation/devicetree/bindings/leds/common.txt
> Documentation/devicetree/bindings/mtd/partition.txt

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Sebastian Reichel @ 2013-12-12 10:05 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Laurent Pinchart, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Archit Taneja,
	Darren Etheridge, Tony Lindgren
In-Reply-To: <52A968BD.20304-l0cyMroinI0@public.gmane.org>

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

On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote:
> > A label property is still an option.
> 
> Hmm, what do you mean? Label as in:
> 
> foo : node {
> };
> 
> Isn't that 'foo' label only visible in DT itself, as a shortcut?

Some driver use a "label" property like this:

foo : node {
    label = "lcd";

    ...
};

See for example

Documentation/devicetree/bindings/leds/common.txt
Documentation/devicetree/bindings/mtd/partition.txt

-- Sebastian

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

^ permalink raw reply

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Tomi Valkeinen @ 2013-12-12  8:54 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <1824203.Ui6fpnZVmO@avalon>

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

Hi,

On 2013-12-12 02:39, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Wednesday 04 December 2013 14:28:27 Tomi Valkeinen wrote:
>> Hi,
>>
>> Here's a new version for DT support to OMAP Display Subsystem. See
>> http://article.gmane.org/gmane.linux.ports.arm.omap/102689 for the intro of
>> the previous version, which contains thoughts about the related problems.
>>
>> The major change in this version is the use of V4L2 and CDF style
>> port/endpoint style in the DT data.
> 
> That's great, and I fully support that. This also calls for refactoring the 
> V4L2 DT bindings and support code to share them with display devices. A 
> bikeshedding question is where to put the common code.

Thanks very much for review!

I know drivers/video is in practice "fbdev", but drivers/video (the
words) sound like the best place for things related to video.

We should establish a committee to ponder this important question.

Btw, I don't know if you noticed (or did I mention it somewhere): I use
a bit shortened form of the V4L2 bindings. If there's just one endpoint,
I omit the port node. I think that's always unambiguous (compared to
also omitting the endpoint node, and having the properties in the main
node, as was mentioned in some other thread).

I also don't supply the same data for both endpoints, when the data is
about the link. E.g. I think the V4L2 binding doc suggests to supply
things like bus-width for both endpoints. I only supply the data for the
endpoint that uses that data. With some encoders/panels the same data
could be needed on both ends, but not in these patches.

>> However, note that even if the DT data contains proper port & endpoint data,
>> the drivers only use the first endpoint. This is to simplify the patches, as
>> adding full support for the ports and endpoints to the drivers will be a big
>> task.
> 
> That's perfectly fine, there's no need to implement unused features just for 
> the sake of it, as long as the bindings are forward-compatible in that 
> respect.
> 
>> This approach still works with more or less all the boards, as the only
>> cases where the simpler model is an issue are the boards with multiple
>> display devices connected to a single output.
>>
>> Laurent, I'd appreciate if you could have a look at the .dts changes, to see
>> if there's anything that's clearly not CDF compatible.
> 
> I've quickly reviewed the patch set, concentrating on the .dts changes. 
> Overall it looks good to me, but I suspect that we will need quite some time 
> to finalize standard bindings as there's lots of small details that need to be 
> taken care of.

Yes. I want to get this forward as it's becoming too much pain to manage
things without DSS DT support, when the rest of the OMAP platform code
is using DT.

The most important thing in the DSS DT bindings for now is that they
should contain enough information so that any future DT bindings changes
can be handled with a boot-time conversion code.

Actually, even the endpoint/port stuff is extra, as that could be
deduced even from the "video-source" method I used in earlier DT versions.

> The major point that bothers we, as explained in my reply to one of the 
> patches, is that I feel like the multi-device model (virtual DSS core + DSS 
> modules) might not accurately represent the hardware. Departing from it 
> probably sounds scary but might not be such a large change.

There are actually two separate things here. One is the multi-device
model, having separate device for each DSS submodule. The other is
having the 'omapdss' "virtual" platform device.

Note that the "dss_core" device is a real HW block, and depicted as the
"dss" node in DT data, whereas "omapdss" device is not present in the DT
data, and is created dynamically at boot time.

The omapdss device is a legacy thing, but nowadays it is mainly used to
pass platform data. Removing omapdss device is not easy, but the only
questions around that is how to implement the required platform-data
functionalities, and it doesn't affect the DT data.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information
From: Tomi Valkeinen @ 2013-12-12  8:38 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tony Lindgren, linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge
In-Reply-To: <2364383.8GWpsWSoJu@avalon>

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

On 2013-12-12 01:44, Laurent Pinchart wrote:

>> The DSS subdevices depend on the dss_core. dss_core has to be powered up
>> for any of the subdevices to work. This is done automatically by the
>> runtime PM when the subdevices are children of the dss_core.
> 
> I'd like to get a clearer picture of the hardware here. The OMAP3 ISP is 
> organized in a seemingly similar way, with the hardware subdivided in high-
> level more-or-less independent modules. However, from a system point of view, 
> the ISP submodules are not standalone: they're part of the same power domain 
> as the ISP core, with subclocks management and interrupts being handled by the 
> ISP core. For those reasons we've modeled the ISP as a single platform device.
> 
> Are the DSS submodules really independent, or are they organized similarly ? 
> For instance do they each have a clock handled by the OMAP core clock IP, or 
> are the clocks gated by the DSS core ? Do they have interrupts routed to the 
> GIC, or does the DSS core driver demux the interrupts ?

The DSS is "interesting". The TRM for various OMAP versions are the best
source of information, there's integration section in the very beginning
of the DSS chapter.

We have the main dss_core (just DSS in the TRM, but for clarity we use
dss_core) module, which is kind of a wrapper/glue for all the
submodules. dss_core contains things like controlling muxes for signals
between submodules, or clocks coming from outside. And there's the DSS
power domain, containing all the DSS modules.

Then we have DISPC, which reads the pixel data, manipulates it, and
outputs raw RGB data to encoder submodules.

Then we have DSI, HDMI, RFBI, VENC encoder submodules. They all have
separate address spaces, some have dedicated PLLs, PHYs, and interrupts.

Then DPI, which I think is mostly just level shifters. It's really just
a port, as you say.

SDI is a bit unclear to me. The registers for it are in the dss_core.
There's only a few registers, but it does have a PHY and a PLL. But I
guess it's also more of a port than a separate module.

As for the clocks, I'm not sure what the actual point is that you want
to clarify. DSS gets one "main" func clock from PRCM, which is used by
DISPC and in some cases other submodules. But then we have dedicated DSI
and HDMI PLLs, which, at least in DSI's case, can be used to fully
satisfy DSI's clock needs. The PLLs can also be used for DISPC, so the
PRCM clock is not needed in all cases.

The interrupts, then. In OMAP4, DISPC, DSI1, DSI2 and HDMI each have
their own interrupt line. In OMAP3, DISPC and DSI shared the same
interrupt line. But in both OMAP4 and OMAP3 DISPC and DSI interrupt
status/enable is handled via the respective IP.

The DSS submodules also are not really designed together. For example,
the HDMI IP is from one vendor, not TI. And the HDMI IP is different in
OMAP4 and OMAP5. Most of the DSS IPs are, I believe, from TI. But it's
not like all the IPs were designed to work together, that's why we have
wrappers/glue blocks (e.g. around HDMI).

So, are they independent? I don't know =). I think they lean on the
independent side. dss_core is always needed for the submodules to work,
but for example DSI could be used without DISPC, using system DMA to
transfer data from memory to DSI. Not a very useful thing to do, but
still, there are dedicated DMA channels for that.

> If the submodules are not independent, would it make sense to have a single DT 
> node that would be matched with the DSS core driver ? You could list 
> information about the submodules in subnodes, and possibly create platform 
> devices internally in the DSS core, but a single platform device would be 
> instantiated from DT, and the DSS core wouldn't need a "simple-bus" compatible 
> string. My gut feeling is that this would be a better representation of the 
> hardware, but I might not known enough about the DSS and be completely wrong.

I have been wondering about this for a long time. The DSS modules have
dependencies, and splitting them into separate devices/drivers brings
the issue of probe order. We side-step that by having the virtual
omapdss driver add the drivers for DSS modules in proper order.

But then, I feel that they are quite independent and probably should be
separate devices. And we've had omap hwmods, which I believe force us to
have separate devices (although afaik hwmods are going away).

>>> BTW, for v3.15, I'm hoping to do patches where we deprecate ti,hwmods
>>> property and do the lookup based on the compatible property instead ;)
>>> So from that point of view we need to get the device mapping right in
>>> the .dtsi files, and don't want to start mixing up separate devices into
>>> single .dtsi entry.
>>
>> Hmm, was that just a general comment, or something that affects the DSS
>> DT data I have in my patch? As far as I understand, the DSS nodes
>> reflect the current hwmods correctly.
>>
>> With the exception that DPI and SDI do not have a matching hwmod, as
>> they are really part of dss_core/dispc. They are separate nodes as they
>> are "video outputs" the same way as the other subnodes.
>>
>> I could perhaps remove the DPI and SDI nodes, and have them as direct
>> video ports from DISPC, but... That's easier said than done.
> 
> DPI and SDI indeed seem like ports to me, node devices. Have you given the 
> implementation a thought ? How difficult would it be ?

I have not though too much about the implementation. I'll spend some
time on that to see how it goes.

There's also the question where do the ports belong to. DISPC outputs
the pixels.

For DPI, I don't see dss_core really doing anything.

For SDI, the dss_core contains the control for the SDI PLL and PHY. But
SDI PLL and PHY are not parts of dss_core, just the control is done via
dss_core.

 Tomi



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

^ permalink raw reply

* [PATCH] video: mxsfb: fix broken video mode selection
From: Lothar Waßmann @ 2013-12-12  8:35 UTC (permalink / raw)
  To: linux-arm-kernel

currently the driver re-implements the code found in
of_get_videomode() except for the fact that the latter honors the
'native-mode' property to select a spcific video timing from the list
of possible timings. The driver builds up a list of all video timings,
but uses only the last mode from the list anyway. While building the
list it incorrectly OR's the 'pixelclk-active' and 'de-active' flags
of all modes into single flags, possibly leading to a wrong pixelclock
or data-enable polarity setting.

Fix this by using the of_get_videomode() directly with the
OF_USE_NATIVE_MODE flag.

Since all current dts files only have one entry in their
display-timings node, this bug was not apparent and the fix does not
change the driver's behaviour for the current users.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/video/mxsfb.c |  126 ++++++++++++++++++++----------------------------
 1 files changed, 53 insertions(+), 73 deletions(-)

diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 27197a8..accf48a2 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -49,6 +49,7 @@
 #include <linux/fb.h>
 #include <linux/regulator/consumer.h>
 #include <video/of_display_timing.h>
+#include <video/of_videomode.h>
 #include <video/videomode.h>
 
 #define REG_SET	4
@@ -297,7 +298,7 @@ static int mxsfb_check_var(struct fb_var_screeninfo *var,
 		}
 		break;
 	default:
-		pr_debug("Unsupported colour depth: %u\n", var->bits_per_pixel);
+		pr_err("Unsupported colour depth: %u\n", var->bits_per_pixel);
 		return -EINVAL;
 	}
 
@@ -426,7 +427,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		ctrl |= CTRL_SET_WORD_LENGTH(3);
 		switch (host->ld_intf_width) {
 		case STMLCDIF_8BIT:
-			dev_dbg(&host->pdev->dev,
+			dev_err(&host->pdev->dev,
 					"Unsupported LCD bus width mapping\n");
 			return -EINVAL;
 		case STMLCDIF_16BIT:
@@ -439,7 +440,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		writel(CTRL1_SET_BYTE_PACKAGING(0x7), host->base + LCDC_CTRL1);
 		break;
 	default:
-		dev_dbg(&host->pdev->dev, "Unhandled color depth of %u\n",
+		dev_err(&host->pdev->dev, "Unhandled color depth of %u\n",
 				fb_info->var.bits_per_pixel);
 		return -EINVAL;
 	}
@@ -589,7 +590,8 @@ static struct fb_ops mxsfb_ops = {
 	.fb_imageblit = cfb_imageblit,
 };
 
-static int mxsfb_restore_mode(struct mxsfb_info *host)
+static int mxsfb_restore_mode(struct mxsfb_info *host,
+			struct fb_videomode *vmode)
 {
 	struct fb_info *fb_info = &host->fb_info;
 	unsigned line_count;
@@ -597,7 +599,6 @@ static int mxsfb_restore_mode(struct mxsfb_info *host)
 	unsigned long pa, fbsize;
 	int bits_per_pixel, ofs;
 	u32 transfer_count, vdctrl0, vdctrl2, vdctrl3, vdctrl4, ctrl;
-	struct fb_videomode vmode;
 
 	/* Only restore the mode when the controller is running */
 	ctrl = readl(host->base + LCDC_CTRL);
@@ -611,8 +612,8 @@ static int mxsfb_restore_mode(struct mxsfb_info *host)
 
 	transfer_count = readl(host->base + host->devdata->transfer_count);
 
-	vmode.xres = TRANSFER_COUNT_GET_HCOUNT(transfer_count);
-	vmode.yres = TRANSFER_COUNT_GET_VCOUNT(transfer_count);
+	vmode->xres = TRANSFER_COUNT_GET_HCOUNT(transfer_count);
+	vmode->yres = TRANSFER_COUNT_GET_VCOUNT(transfer_count);
 
 	switch (CTRL_GET_WORD_LENGTH(ctrl)) {
 	case 0:
@@ -628,40 +629,39 @@ static int mxsfb_restore_mode(struct mxsfb_info *host)
 
 	fb_info->var.bits_per_pixel = bits_per_pixel;
 
-	vmode.pixclock = KHZ2PICOS(clk_get_rate(host->clk) / 1000U);
-	vmode.hsync_len = get_hsync_pulse_width(host, vdctrl2);
-	vmode.left_margin = GET_HOR_WAIT_CNT(vdctrl3) - vmode.hsync_len;
-	vmode.right_margin = VDCTRL2_GET_HSYNC_PERIOD(vdctrl2) - vmode.hsync_len -
-		vmode.left_margin - vmode.xres;
-	vmode.vsync_len = VDCTRL0_GET_VSYNC_PULSE_WIDTH(vdctrl0);
+	vmode->pixclock = KHZ2PICOS(clk_get_rate(host->clk) / 1000U);
+	vmode->hsync_len = get_hsync_pulse_width(host, vdctrl2);
+	vmode->left_margin = GET_HOR_WAIT_CNT(vdctrl3) - vmode->hsync_len;
+	vmode->right_margin = VDCTRL2_GET_HSYNC_PERIOD(vdctrl2) -
+		vmode->hsync_len - vmode->left_margin - vmode->xres;
+	vmode->vsync_len = VDCTRL0_GET_VSYNC_PULSE_WIDTH(vdctrl0);
 	period = readl(host->base + LCDC_VDCTRL1);
-	vmode.upper_margin = GET_VERT_WAIT_CNT(vdctrl3) - vmode.vsync_len;
-	vmode.lower_margin = period - vmode.vsync_len - vmode.upper_margin - vmode.yres;
+	vmode->upper_margin = GET_VERT_WAIT_CNT(vdctrl3) - vmode->vsync_len;
+	vmode->lower_margin = period - vmode->vsync_len -
+		vmode->upper_margin - vmode->yres;
 
-	vmode.vmode = FB_VMODE_NONINTERLACED;
+	vmode->vmode = FB_VMODE_NONINTERLACED;
 
-	vmode.sync = 0;
+	vmode->sync = 0;
 	if (vdctrl0 & VDCTRL0_HSYNC_ACT_HIGH)
-		vmode.sync |= FB_SYNC_HOR_HIGH_ACT;
+		vmode->sync |= FB_SYNC_HOR_HIGH_ACT;
 	if (vdctrl0 & VDCTRL0_VSYNC_ACT_HIGH)
-		vmode.sync |= FB_SYNC_VERT_HIGH_ACT;
+		vmode->sync |= FB_SYNC_VERT_HIGH_ACT;
 
 	pr_debug("Reconstructed video mode:\n");
 	pr_debug("%dx%d, hsync: %u left: %u, right: %u, vsync: %u, upper: %u, lower: %u\n",
-			vmode.xres, vmode.yres,
-			vmode.hsync_len, vmode.left_margin, vmode.right_margin,
-			vmode.vsync_len, vmode.upper_margin, vmode.lower_margin);
-	pr_debug("pixclk: %ldkHz\n", PICOS2KHZ(vmode.pixclock));
-
-	fb_add_videomode(&vmode, &fb_info->modelist);
+		vmode->xres, vmode->yres, vmode->hsync_len, vmode->left_margin,
+		vmode->right_margin, vmode->vsync_len, vmode->upper_margin,
+		vmode->lower_margin);
+	pr_debug("pixclk: %ldkHz\n", PICOS2KHZ(vmode->pixclock));
 
 	host->ld_intf_width = CTRL_GET_BUS_WIDTH(ctrl);
 	host->dotclk_delay = VDCTRL4_GET_DOTCLK_DLY(vdctrl4);
 
-	fb_info->fix.line_length = vmode.xres * (bits_per_pixel >> 3);
+	fb_info->fix.line_length = vmode->xres * (bits_per_pixel >> 3);
 
 	pa = readl(host->base + host->devdata->cur_buf);
-	fbsize = fb_info->fix.line_length * vmode.yres;
+	fbsize = fb_info->fix.line_length * vmode->yres;
 	if (pa < fb_info->fix.smem_start)
 		return -EINVAL;
 	if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len)
@@ -681,18 +681,17 @@ static int mxsfb_restore_mode(struct mxsfb_info *host)
 	return 0;
 }
 
-static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host)
+static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
+				struct fb_videomode *vmode)
 {
 	struct fb_info *fb_info = &host->fb_info;
 	struct fb_var_screeninfo *var = &fb_info->var;
 	struct device *dev = &host->pdev->dev;
 	struct device_node *np = host->pdev->dev.of_node;
 	struct device_node *display_np;
-	struct device_node *timings_np;
-	struct display_timings *timings;
+	struct videomode vm;
 	u32 width;
-	int i;
-	int ret = 0;
+	int ret;
 
 	display_np = of_parse_phandle(np, "display", 0);
 	if (!display_np) {
@@ -732,54 +731,35 @@ static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host)
 		goto put_display_node;
 	}
 
-	timings = of_get_display_timings(display_np);
-	if (!timings) {
-		dev_err(dev, "failed to get display timings\n");
-		ret = -ENOENT;
+	ret = of_get_videomode(display_np, &vm, OF_USE_NATIVE_MODE);
+	if (ret) {
+		dev_err(dev, "failed to get videomode from DT\n");
 		goto put_display_node;
 	}
 
-	timings_np = of_find_node_by_name(display_np,
-					  "display-timings");
-	if (!timings_np) {
-		dev_err(dev, "failed to find display-timings node\n");
-		ret = -ENOENT;
+	ret = fb_videomode_from_videomode(&vm, vmode);
+	if (ret < 0)
 		goto put_display_node;
-	}
 
-	for (i = 0; i < of_get_child_count(timings_np); i++) {
-		struct videomode vm;
-		struct fb_videomode fb_vm;
-
-		ret = videomode_from_timings(timings, &vm, i);
-		if (ret < 0)
-			goto put_timings_node;
-		ret = fb_videomode_from_videomode(&vm, &fb_vm);
-		if (ret < 0)
-			goto put_timings_node;
-
-		if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
-			host->sync |= MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
-		if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
-			host->sync |= MXSFB_SYNC_DOTCLK_FALLING_ACT;
-		fb_add_videomode(&fb_vm, &fb_info->modelist);
-	}
+	if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
+		host->sync |= MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
+	if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+		host->sync |= MXSFB_SYNC_DOTCLK_FALLING_ACT;
 
-put_timings_node:
-	of_node_put(timings_np);
 put_display_node:
 	of_node_put(display_np);
 	return ret;
 }
 
-static int mxsfb_init_fbinfo(struct mxsfb_info *host)
+static int mxsfb_init_fbinfo(struct mxsfb_info *host,
+			struct fb_videomode *vmode)
 {
+	int ret;
 	struct fb_info *fb_info = &host->fb_info;
 	struct fb_var_screeninfo *var = &fb_info->var;
 	dma_addr_t fb_phys;
 	void *fb_virt;
 	unsigned fb_size;
-	int ret;
 
 	fb_info->fbops = &mxsfb_ops;
 	fb_info->flags = FBINFO_FLAG_DEFAULT | FBINFO_READS_FAST;
@@ -789,7 +769,7 @@ static int mxsfb_init_fbinfo(struct mxsfb_info *host)
 	fb_info->fix.visual = FB_VISUAL_TRUECOLOR,
 	fb_info->fix.accel = FB_ACCEL_NONE;
 
-	ret = mxsfb_init_fbinfo_dt(host);
+	ret = mxsfb_init_fbinfo_dt(host, vmode);
 	if (ret)
 		return ret;
 
@@ -810,7 +790,7 @@ static int mxsfb_init_fbinfo(struct mxsfb_info *host)
 	fb_info->screen_base = fb_virt;
 	fb_info->screen_size = fb_info->fix.smem_len = fb_size;
 
-	if (mxsfb_restore_mode(host))
+	if (mxsfb_restore_mode(host, vmode))
 		memset(fb_virt, 0, fb_size);
 
 	return 0;
@@ -850,7 +830,7 @@ static int mxsfb_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct mxsfb_info *host;
 	struct fb_info *fb_info;
-	struct fb_modelist *modelist;
+	struct fb_videomode *mode;
 	int ret;
 
 	if (of_id)
@@ -862,6 +842,11 @@ static int mxsfb_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	mode = devm_kzalloc(&pdev->dev, sizeof(struct fb_videomode),
+			GFP_KERNEL);
+	if (mode = NULL)
+		return -ENOMEM;
+
 	host = to_imxfb_host(fb_info);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -893,15 +878,11 @@ static int mxsfb_probe(struct platform_device *pdev)
 		goto fb_release;
 	}
 
-	INIT_LIST_HEAD(&fb_info->modelist);
-
-	ret = mxsfb_init_fbinfo(host);
+	ret = mxsfb_init_fbinfo(host, mode);
 	if (ret != 0)
 		goto fb_release;
 
-	modelist = list_first_entry(&fb_info->modelist,
-			struct fb_modelist, list);
-	fb_videomode_to_var(&fb_info->var, &modelist->mode);
+	fb_videomode_to_var(&fb_info->var, mode);
 
 	/* init the color fields */
 	mxsfb_check_var(&fb_info->var, fb_info);
@@ -927,7 +908,6 @@ static int mxsfb_probe(struct platform_device *pdev)
 fb_destroy:
 	if (host->enabled)
 		clk_disable_unprepare(host->clk);
-	fb_destroy_modelist(&fb_info->modelist);
 fb_release:
 	framebuffer_release(fb_info);
 
-- 
1.7.2.5


^ permalink raw reply related

* Re: [PATCH 1/3] arm: omap2: Export devconf1 bypass and acbias.
From: Belisko Marek @ 2013-12-12  8:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131210224659.GY13171@atomide.com>

Hi Tony,

On Tue, Dec 10, 2013 at 11:46 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Belisko Marek <marek.belisko@gmail.com> [131210 14:13]:
>> On Tue, Nov 12, 2013 at 12:31 AM, Tony Lindgren <tony@atomide.com> wrote:
>> >
>> > It would be best to set it up as omap-ctrl.c driver under drivers
>> > somewhere with few functions exported for DSS and MMC drivers.
>>
>> I create small dummy driver based on phy-omap-control which can be
>> used but before sending patch (with more updates) I would like to get
>> some feedback if my direction is correct.
>
> Cool thanks. Yeah what you have can easily be combined with the patches
> Balaji has posted to make HSMMC use drivers/mfd/syscon.c via regmap
> for the SCM register access. Maybe take a look at the work in progress
> patches in thread:
>
> [PATCH v4 0/7] mmc: omap_hsmmc: pbias dt and cleanup
>
> And also see my comments regarding using the SCM GENERAL register area
> as base for the syscon.c driver. That should work for your driver too,
> right? And then you can access the SYSCON1 register that way from your
> consumer driver ;)
If I understand correclty I can use syscon driver (it will have in
range also devconf1 register) ad get rid of my custom driver
and then get regmap from syscon and update bits that I need for venc, right?
>
> Regards,
>
> Tony

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com

^ permalink raw reply

* Re: [PATCH 10/26] OMAPDSS: add of helpers
From: Tomi Valkeinen @ 2013-12-12  7:48 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren
In-Reply-To: <4533892.HJaRcDt4Q8@avalon>

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

On 2013-12-12 01:19, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Wednesday 04 December 2013 14:28:37 Tomi Valkeinen wrote:
>> Add helpers to get ports and endpoints from DT data.
>>
>> While all the functions in dss-of.c might be useful for panel drivers if
>> they need to parse full port/endpoint data, at the moment we only need a
>> few of them outside dss-of.c, so only those functions are exported.
> 
> I totally understand that it was easier to add this code to the OMAP DSS 
> driver, but I believe we should refactor the existing drivers/media/v4l2-
> core/v4l2-of.c and move it to a non V4L2-specific location (what about 
> drivers/media ?) sooner rather than later. That's on my to-do list for 
> Saturday.

I agree. I just didn't want to go that way yet =).

Have you thought of the API? You had one version in your CDF series, but
I think that was a bit too limited (I don't remember right now how), so
I wrote my own versions.

What I tried to do here is to add simple ways for the drivers to iterate
the ports and endpoints with omapdss_of_get_next_port and
omapdss_of_get_next_endpoint.

But I'm not sure what the use pattern would be. If in most of the cases
the driver always goes through all the ports and all the endpoints, we
could as well have a helper function that goes through all the endpoints
in all the ports, and returns both the port and endpoint for each iteration.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Tomi Valkeinen @ 2013-12-12  7:41 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren
In-Reply-To: <10507960.9DXH4TSv2E@avalon>

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

On 2013-12-12 01:56, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Thursday 12 December 2013 00:13:01 Laurent Pinchart wrote:
>> On Wednesday 04 December 2013 14:28:33 Tomi Valkeinen wrote:
>>> To avoid the need for a "nickname" property for each display, change
>>> the display registration so that the display's alias (i.e. "display0"
>>> etc) will be used for the dssdev->name if the display driver didn't
>>> provide a name.
>>>
>>> This means that when booting with board files, we will have more
>>> descriptive names for displays, like "lcd1", "hdmi".
>>
>> Where are those names used ? Are they reported to userspace, or limited to
>> kernel internal use only ?

They are visible via omapdss's sysfs, when using omapfb. They are used
for debug prints and by the user for selecting the default display and
display modes via kernel cmdline, and when he sets the video pipeline
routing. So changing them could be considered breaking the API, but...

With omapdrm the sysfs files do not exist, and I think omapdrm doesn't
use them (except maybe for some debug prints).

>>> With DT we'll only have "display0", etc. But as there are no "nicknames"
>>> for things like serials ports either, I hope we will do fine with this
>>
>> Just a random thought, maybe the aliases node could help here.
> 
> I should have read the next patches before replying, sorry :-)
> 
>> I'm not sure what rules govern its usage. Adding labels to display DT nodes
>> could be an option too.

Using of_alias_get_id() means that the alias is in the form "nameX"
where X is a number.

> A label property is still an option.

Hmm, what do you mean? Label as in:

foo : node {
};

Isn't that 'foo' label only visible in DT itself, as a shortcut?

 Tomi



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

^ permalink raw reply

* Re: [PATCH 05/26] ARM: OMAP2+: add omapdss_init_of()
From: Tomi Valkeinen @ 2013-12-12  7:30 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren
In-Reply-To: <2199639.FbGhznOLP9@avalon>

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

On 2013-12-12 01:10, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Wednesday 04 December 2013 14:28:32 Tomi Valkeinen wrote:
>> omapdss driver uses a omapdss platform device to pass platform specific
>> function pointers and DSS hardware version from the arch code to the
>> driver. This device is needed also when booting with DT.
>>
>> This patch adds omapdss_init_of() function, called from board-generic at
>> init time, which creates the omapdss device.
> 
> Is this a temporary solution that you plan to later replace with DT-only 
> device instantiation ?

It's a long term task to remove the "virtual" omapdss device. Removing
the platform data that we pass has been very difficult.

For example, we need to get the OMAP revision to know which OMAP DSS
hardware we have. We can't get that information from the DSS hardware
(thank you, HW designers! ;).

Another is DSI pin muxing. I think we need a new pinmuxing driver for
that, or maybe change pinmux-single quite a bit. The DSI pinmuxing is
very simple, but the register fields are varying lengths and at varying
positions, so pinmux-single doesn't work for it.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Laurent Pinchart @ 2013-12-12  0:39 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <1386160133-24026-1-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

On Wednesday 04 December 2013 14:28:27 Tomi Valkeinen wrote:
> Hi,
> 
> Here's a new version for DT support to OMAP Display Subsystem. See
> http://article.gmane.org/gmane.linux.ports.arm.omap/102689 for the intro of
> the previous version, which contains thoughts about the related problems.
> 
> The major change in this version is the use of V4L2 and CDF style
> port/endpoint style in the DT data.

That's great, and I fully support that. This also calls for refactoring the 
V4L2 DT bindings and support code to share them with display devices. A 
bikeshedding question is where to put the common code.

> However, note that even if the DT data contains proper port & endpoint data,
> the drivers only use the first endpoint. This is to simplify the patches, as
> adding full support for the ports and endpoints to the drivers will be a big
> task.

That's perfectly fine, there's no need to implement unused features just for 
the sake of it, as long as the bindings are forward-compatible in that 
respect.

> This approach still works with more or less all the boards, as the only
> cases where the simpler model is an issue are the boards with multiple
> display devices connected to a single output.
> 
> Laurent, I'd appreciate if you could have a look at the .dts changes, to see
> if there's anything that's clearly not CDF compatible.

I've quickly reviewed the patch set, concentrating on the .dts changes. 
Overall it looks good to me, but I suspect that we will need quite some time 
to finalize standard bindings as there's lots of small details that need to be 
taken care of.

The major point that bothers we, as explained in my reply to one of the 
patches, is that I feel like the multi-device model (virtual DSS core + DSS 
modules) might not accurately represent the hardware. Departing from it 
probably sounds scary but might not be such a large change.

> The patches can also be found from:
> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git work/dss-dt
> 
> A few notes:
> 
> - The DT data are added separately in the end of .dts files for clarity. In
>   the final version I will move them to appropriate places in the .dts
>   files.
> 
> - No binding documentation. I will add them for the next version, if there
>   are no major changes needed. Hopefully the bindings are quite self-
>   explanatory for people with understanding of the hardware in question.

My plan is to split the connection information from the V4L2 bindings and make 
that a separate document. If all goes well I should be able to spend Saturday 
on this.

> - The connectors' compatible strings are "ti,xxx". As there's nothing TI
>   specific there, I think I will rename them to be without "ti".

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Laurent Pinchart @ 2013-12-11 23:56 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren
In-Reply-To: <37483030.ghHfzYFSnL@avalon>

Hi Tomi,

On Thursday 12 December 2013 00:13:01 Laurent Pinchart wrote:
> On Wednesday 04 December 2013 14:28:33 Tomi Valkeinen wrote:
> > To avoid the need for a "nickname" property for each display, change
> > the display registration so that the display's alias (i.e. "display0"
> > etc) will be used for the dssdev->name if the display driver didn't
> > provide a name.
> > 
> > This means that when booting with board files, we will have more
> > descriptive names for displays, like "lcd1", "hdmi".
> 
> Where are those names used ? Are they reported to userspace, or limited to
> kernel internal use only ?
> 
> > With DT we'll only have "display0", etc. But as there are no "nicknames"
> > for things like serials ports either, I hope we will do fine with this
>
> Just a random thought, maybe the aliases node could help here.

I should have read the next patches before replying, sorry :-)

> I'm not sure what rules govern its usage. Adding labels to display DT nodes
> could be an option too.

A label property is still an option.

> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > ---
> > 
> >  drivers/video/omap2/dss/display.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/video/omap2/dss/display.c
> > b/drivers/video/omap2/dss/display.c index 669a81fdf58e..a946cf7ed00f
> > 100644
> > --- a/drivers/video/omap2/dss/display.c
> > +++ b/drivers/video/omap2/dss/display.c
> > @@ -137,6 +137,9 @@ int omapdss_register_display(struct omap_dss_device
> > *dssdev)
> >  	snprintf(dssdev->alias, sizeof(dssdev->alias),
> >  			"display%d", disp_num_counter++);
> > 
> > +	if (dssdev->name = NULL)
> > +		dssdev->name = dssdev->alias;
> > +
> >  	if (drv && drv->get_resolution = NULL)
> >  		drv->get_resolution = omapdss_default_get_resolution;
> >  	if (drv && drv->get_recommended_bpp = NULL)
-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information
From: Laurent Pinchart @ 2013-12-11 23:44 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Tony Lindgren, linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge
In-Reply-To: <52A5BB65.5050109@ti.com>

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

Hi Tomi,

On Monday 09 December 2013 14:45:25 Tomi Valkeinen wrote:
> On 2013-12-05 19:05, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [131204 04:31]:
> > 
> > Description missing.. But other than that can you please check that
> > the latest patch I posted in thread "[PATCH] ARM: OMAP2+: Fix populating
> > the hwmod data from device" works with this?
> > 
> > The test to do is to remove the related reg, interrupt and dma entries
> > from omap_hwmod_*_data.c, and make sure the related hwmod data is
> > initialized from DT properly.
> 
> I made a quick test with panda, by applying your patch and reverting
> b38911f3472be89551bfca740adf0009562b9873. That only effectively tests
> the DISPC IRQ, but that worked fine.
> 
> > I don't know if it makes sense to have them as children of dss_core, they
> > really all seem to be completely independent devices?
> 
> The DSS subdevices depend on the dss_core. dss_core has to be powered up
> for any of the subdevices to work. This is done automatically by the
> runtime PM when the subdevices are children of the dss_core.

I'd like to get a clearer picture of the hardware here. The OMAP3 ISP is 
organized in a seemingly similar way, with the hardware subdivided in high-
level more-or-less independent modules. However, from a system point of view, 
the ISP submodules are not standalone: they're part of the same power domain 
as the ISP core, with subclocks management and interrupts being handled by the 
ISP core. For those reasons we've modeled the ISP as a single platform device.

Are the DSS submodules really independent, or are they organized similarly ? 
For instance do they each have a clock handled by the OMAP core clock IP, or 
are the clocks gated by the DSS core ? Do they have interrupts routed to the 
GIC, or does the DSS core driver demux the interrupts ?

If the submodules are not independent, would it make sense to have a single DT 
node that would be matched with the DSS core driver ? You could list 
information about the submodules in subnodes, and possibly create platform 
devices internally in the DSS core, but a single platform device would be 
instantiated from DT, and the DSS core wouldn't need a "simple-bus" compatible 
string. My gut feeling is that this would be a better representation of the 
hardware, but I might not known enough about the DSS and be completely wrong.

> > BTW, for v3.15, I'm hoping to do patches where we deprecate ti,hwmods
> > property and do the lookup based on the compatible property instead ;)
> > So from that point of view we need to get the device mapping right in
> > the .dtsi files, and don't want to start mixing up separate devices into
> > single .dtsi entry.
> 
> Hmm, was that just a general comment, or something that affects the DSS
> DT data I have in my patch? As far as I understand, the DSS nodes
> reflect the current hwmods correctly.
> 
> With the exception that DPI and SDI do not have a matching hwmod, as
> they are really part of dss_core/dispc. They are separate nodes as they
> are "video outputs" the same way as the other subnodes.
> 
> I could perhaps remove the DPI and SDI nodes, and have them as direct
> video ports from DISPC, but... That's easier said than done.

DPI and SDI indeed seem like ports to me, node devices. Have you given the 
implementation a thought ? How difficult would it be ?

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply

* Re: [PATCH 10/26] OMAPDSS: add of helpers
From: Laurent Pinchart @ 2013-12-11 23:19 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren
In-Reply-To: <1386160133-24026-11-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

On Wednesday 04 December 2013 14:28:37 Tomi Valkeinen wrote:
> Add helpers to get ports and endpoints from DT data.
> 
> While all the functions in dss-of.c might be useful for panel drivers if
> they need to parse full port/endpoint data, at the moment we only need a
> few of them outside dss-of.c, so only those functions are exported.

I totally understand that it was easier to add this code to the OMAP DSS 
driver, but I believe we should refactor the existing drivers/media/v4l2-
core/v4l2-of.c and move it to a non V4L2-specific location (what about 
drivers/media ?) sooner rather than later. That's on my to-do list for 
Saturday.

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/video/omap2/dss/Makefile |   2 +-
>  drivers/video/omap2/dss/dss-of.c | 160 ++++++++++++++++++++++++++++++++++++
>  include/video/omapdss.h          |   6 ++
>  3 files changed, 167 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/video/omap2/dss/dss-of.c
> 
> diff --git a/drivers/video/omap2/dss/Makefile
> b/drivers/video/omap2/dss/Makefile index d3aa91bdd6a8..8aec8bda27cc 100644
> --- a/drivers/video/omap2/dss/Makefile
> +++ b/drivers/video/omap2/dss/Makefile
> @@ -1,7 +1,7 @@
>  obj-$(CONFIG_OMAP2_DSS) += omapdss.o
>  # Core DSS files
>  omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o \
> -	output.o
> +	output.o dss-of.o
>  # DSS compat layer files
>  omapdss-y += manager.o manager-sysfs.o overlay.o overlay-sysfs.o apply.o \
>  	dispc-compat.o display-sysfs.o
> diff --git a/drivers/video/omap2/dss/dss-of.c
> b/drivers/video/omap2/dss/dss-of.c new file mode 100644
> index 000000000000..9aa61d4bdb3d
> --- /dev/null
> +++ b/drivers/video/omap2/dss/dss-of.c
> @@ -0,0 +1,160 @@
> +/*
> + * Copyright (C) 2013 Texas Instruments
> + * Author: Tomi Valkeinen <tomi.valkeinen@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.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/seq_file.h>
> +
> +#include <video/omapdss.h>
> +
> +#include "dss.h"
> +#include "dss_features.h"
> +
> +static struct device_node *
> +omapdss_of_get_next_port(const struct device_node *parent,
> +			 struct device_node *prev)
> +{
> +	struct device_node *port = NULL;
> +
> +	if (!parent)
> +		return NULL;
> +
> +	if (!prev) {
> +		struct device_node *ports;
> +		/*
> +		 * It's the first call, we have to find a port subnode
> +		 * within this node or within an optional 'ports' node.
> +		 */
> +		ports = of_get_child_by_name(parent, "ports");
> +		if (ports)
> +			parent = ports;
> +
> +		port = of_get_child_by_name(parent, "port");
> +
> +		/* release the 'ports' node */
> +		of_node_put(ports);
> +	} else {
> +		struct device_node *ports;
> +
> +		ports = of_get_parent(prev);
> +		if (!ports)
> +			return NULL;
> +
> +		do {
> +			port = of_get_next_child(ports, prev);
> +			if (!port) {
> +				of_node_put(ports);
> +				return NULL;
> +			}
> +			prev = port;
> +		} while (of_node_cmp(port->name, "port") != 0);
> +	}
> +
> +	return port;
> +}
> +
> +static struct device_node *
> +omapdss_of_get_next_endpoint(const struct device_node *parent,
> +			     struct device_node *prev)
> +{
> +	struct device_node *ep = NULL;
> +
> +	if (!parent)
> +		return NULL;
> +
> +	do {
> +		ep = of_get_next_child(parent, prev);
> +		if (!ep)
> +			return NULL;
> +		prev = ep;
> +	} while (of_node_cmp(ep->name, "endpoint") != 0);
> +
> +	return ep;
> +}
> +
> +static struct device_node *
> +omapdss_of_get_remote_device_node(const struct device_node *node)
> +{
> +	struct device_node *np;
> +	int i;
> +
> +	np = of_parse_phandle(node, "remote-endpoint", 0);
> +
> +	if (!np)
> +		return NULL;
> +
> +	np = of_get_next_parent(np);
> +
> +	for (i = 0; i < 3 && np; ++i) {
> +		struct property *prop;
> +
> +		prop = of_find_property(np, "compatible", NULL);
> +
> +		if (prop)
> +			return np;
> +
> +		np = of_get_next_parent(np);
> +	}
> +
> +	return NULL;
> +}
> +
> +struct device_node *
> +omapdss_of_get_first_endpoint(const struct device_node *parent)
> +{
> +	struct device_node *port;
> +	struct device_node *ep;
> +
> +	port = omapdss_of_get_next_port(parent, NULL);
> +	if (port) {
> +		ep = omapdss_of_get_next_endpoint(port, NULL);
> +		of_node_put(port);
> +	} else {
> +		ep = omapdss_of_get_next_endpoint(parent, NULL);
> +	}
> +
> +	return ep;
> +}
> +EXPORT_SYMBOL_GPL(omapdss_of_get_first_endpoint);
> +
> +struct omap_dss_device *
> +omapdss_of_find_source_for_first_ep(struct device_node *node)
> +{
> +	struct device_node *ep;
> +	struct device_node *src_node;
> +	struct omap_dss_device *src;
> +
> +	ep = omapdss_of_get_first_endpoint(node);
> +	if (!ep)
> +		return ERR_PTR(-EINVAL);
> +
> +	src_node = omapdss_of_get_remote_device_node(ep);
> +
> +	of_node_put(ep);
> +
> +	if (!src_node)
> +		return ERR_PTR(-EINVAL);
> +
> +	src = omap_dss_find_output_by_node(src_node);
> +
> +	of_node_put(src_node);
> +
> +	if (!src)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return src;
> +}
> +EXPORT_SYMBOL_GPL(omapdss_of_find_source_for_first_ep);
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index 3d7c51a6f9ff..c510591df1b8 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -1019,4 +1019,10 @@ static inline bool omapdss_device_is_enabled(struct
> omap_dss_device *dssdev) return dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
>  }
> 
> +struct device_node *
> +omapdss_of_get_first_endpoint(const struct device_node *parent);
> +
> +struct omap_dss_device *
> +omapdss_of_find_source_for_first_ep(struct device_node *node);
> +
>  #endif
-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Laurent Pinchart @ 2013-12-11 23:13 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren
In-Reply-To: <1386160133-24026-7-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

On Wednesday 04 December 2013 14:28:33 Tomi Valkeinen wrote:
> To avoid the need for a "nickname" property for each display, change
> the display registration so that the display's alias (i.e. "display0"
> etc) will be used for the dssdev->name if the display driver didn't
> provide a name.
> 
> This means that when booting with board files, we will have more
> descriptive names for displays, like "lcd1", "hdmi".

Where are those names used ? Are they reported to userspace, or limited to 
kernel internal use only ?

> With DT we'll only have "display0", etc. But as there are no "nicknames" for
> things like serials ports either, I hope we will do fine with this approach.

Just a random thought, maybe the aliases node could help here. I'm not sure 
what rules govern its usage. Adding labels to display DT nodes could be an 
option too.

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/video/omap2/dss/display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/video/omap2/dss/display.c
> b/drivers/video/omap2/dss/display.c index 669a81fdf58e..a946cf7ed00f 100644
> --- a/drivers/video/omap2/dss/display.c
> +++ b/drivers/video/omap2/dss/display.c
> @@ -137,6 +137,9 @@ int omapdss_register_display(struct omap_dss_device
> *dssdev) snprintf(dssdev->alias, sizeof(dssdev->alias),
>  			"display%d", disp_num_counter++);
> 
> +	if (dssdev->name = NULL)
> +		dssdev->name = dssdev->alias;
> +
>  	if (drv && drv->get_resolution = NULL)
>  		drv->get_resolution = omapdss_default_get_resolution;
>  	if (drv && drv->get_recommended_bpp = NULL)
-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH 05/26] ARM: OMAP2+: add omapdss_init_of()
From: Laurent Pinchart @ 2013-12-11 23:10 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren
In-Reply-To: <1386160133-24026-6-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

On Wednesday 04 December 2013 14:28:32 Tomi Valkeinen wrote:
> omapdss driver uses a omapdss platform device to pass platform specific
> function pointers and DSS hardware version from the arch code to the
> driver. This device is needed also when booting with DT.
> 
> This patch adds omapdss_init_of() function, called from board-generic at
> init time, which creates the omapdss device.

Is this a temporary solution that you plan to later replace with DT-only 
device instantiation ?

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  arch/arm/mach-omap2/board-generic.c |  2 ++
>  arch/arm/mach-omap2/common.h        |  2 ++
>  arch/arm/mach-omap2/display.c       | 62 ++++++++++++++++++++++++++++++++++
>  3 files changed, 66 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/board-generic.c
> b/arch/arm/mach-omap2/board-generic.c index 19f1652e94cf..0e06771d7bee
> 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -36,6 +36,8 @@ static struct of_device_id omap_dt_match_table[]
> __initdata = { static void __init omap_generic_init(void)
>  {
>  	pdata_quirks_init(omap_dt_match_table);
> +
> +	omapdss_init_of();
>  }
> 
>  #ifdef CONFIG_SOC_OMAP2420
> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index f7644febee81..48e9cd34cae0 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -308,5 +308,7 @@ extern int omap_dss_reset(struct omap_hwmod *);
>  /* SoC specific clock initializer */
>  extern int (*omap_clk_init)(void);
> 
> +int __init omapdss_init_of(void);
> +
>  #endif /* __ASSEMBLER__ */
>  #endif /* __ARCH_ARM_MACH_OMAP2PLUS_COMMON_H */
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index a4e536b11ec9..3279afc5f0b5 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -23,6 +23,8 @@
>  #include <linux/clk.h>
>  #include <linux/err.h>
>  #include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> 
>  #include <video/omapdss.h>
>  #include "omap_hwmod.h"
> @@ -592,3 +594,63 @@ int omap_dss_reset(struct omap_hwmod *oh)
> 
>  	return r;
>  }
> +
> +int __init omapdss_init_of(void)
> +{
> +	int r;
> +	enum omapdss_version ver;
> +
> +	static struct omap_dss_board_info board_data = {
> +		.dsi_enable_pads = omap_dsi_enable_pads,
> +		.dsi_disable_pads = omap_dsi_disable_pads,
> +		.get_context_loss_count = omap_pm_get_dev_context_loss_count,
> +		.set_min_bus_tput = omap_dss_set_min_bus_tput,
> +	};
> +
> +	ver = omap_display_get_version();
> +
> +	if (ver = OMAPDSS_VER_UNKNOWN) {
> +		pr_err("DSS not supported on this SoC\n");
> +		return -ENODEV;
> +	}
> +
> +	board_data.version = ver;
> +
> +	omap_display_device.dev.platform_data = &board_data;
> +
> +	r = platform_device_register(&omap_display_device);
> +	if (r < 0) {
> +		pr_err("Unable to register omapdss device\n");
> +		return r;
> +	}
> +
> +	/* create DRM device */
> +	r = omap_init_drm();
> +	if (r < 0) {
> +		pr_err("Unable to register omapdrm device\n");
> +		return r;
> +	}
> +
> +	/* create vrfb device */
> +	r = omap_init_vrfb();
> +	if (r < 0) {
> +		pr_err("Unable to register omapvrfb device\n");
> +		return r;
> +	}
> +
> +	/* create FB device */
> +	r = omap_init_fb();
> +	if (r < 0) {
> +		pr_err("Unable to register omapfb device\n");
> +		return r;
> +	}
> +
> +	/* create V4L2 display device */
> +	r = omap_init_vout();
> +	if (r < 0) {
> +		pr_err("Unable to register omap_vout device\n");
> +		return r;
> +	}
> +
> +	return 0;
> +}
-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH] ARM: OMAPFB: panel-sony-acx565akm: fix missing unlock in acx565akm_panel_power_on()
From: Tomi Valkeinen @ 2013-12-11 14:29 UTC (permalink / raw)
  To: Wei Yongjun, plagnioj, aaro.koskinen; +Cc: yongjun_wei, linux-omap, linux-fbdev
In-Reply-To: <CAPgLHd_asZkqBEXyx611=NOMfcaR18jZ8Yxkt5yhg7Lbng93MA@mail.gmail.com>

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

Hi,

On 2013-12-06 14:55, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> Add the missing unlock before return from function
> acx565akm_panel_power_on() in the error handling case.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

A fix for this has already been merged:
c37dd677988ca50bc8bc60ab5ab053720583c168 (ARM: OMAPFB:
panel-sony-acx565akm: fix bad unlock balance)

 Tomi



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

^ permalink raw reply

* Re: [PATCH 13/15] fbdev: sh-mobile-lcdcfb: Enable driver compilation with COMPILE_TEST
From: Laurent Pinchart @ 2013-12-11 12:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1385515117-23664-14-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

Hi Jean-Christophe and Tomi,

Could you please pick this patch up for v3.14 ?

On Wednesday 27 November 2013 02:18:35 Laurent Pinchart wrote:
> This helps increasing build testing coverage.
> 
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Simon Horman <horms@verge.net.au>
> ---
>  drivers/video/Kconfig | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 4f2e1b3..2aceb08 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -10,7 +10,8 @@ config HAVE_FB_ATMEL
> 
>  config SH_MIPI_DSI
>  	tristate
> -	depends on (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
> +	depends on HAVE_CLK
> +	depends on SUPERH || ARCH_SHMOBILE || COMPILE_TEST
> 
>  config SH_LCD_MIPI_DSI
>  	bool
> @@ -1997,7 +1998,8 @@ config FB_W100
> 
>  config FB_SH_MOBILE_LCDC
>  	tristate "SuperH Mobile LCDC framebuffer support"
> -	depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
> +	depends on FB && HAVE_CLK
> +	depends on SUPERH || ARCH_SHMOBILE || COMPILE_TEST
>  	select FB_SYS_FILLRECT
>  	select FB_SYS_COPYAREA
>  	select FB_SYS_IMAGEBLIT
> @@ -2484,7 +2486,7 @@ endif
> 
>  config FB_SH_MOBILE_MERAM
>  	tristate "SuperH Mobile MERAM read ahead support"
> -	depends on (SUPERH || ARCH_SHMOBILE)
> +	depends on SUPERH || ARCH_SHMOBILE || COMPILE_TEST
>  	select GENERIC_ALLOCATOR
>  	---help---
>  	  Enable MERAM support for the SuperH controller.
-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH 1/3] arm: omap2: Export devconf1 bypass and acbias.
From: Tony Lindgren @ 2013-12-10 22:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAAfyv36MaVqrmQxpqDu-ciO9GNJK3pJkFZohEvd1BOLOWWEOhw@mail.gmail.com>

* Belisko Marek <marek.belisko@gmail.com> [131210 14:13]:
> On Tue, Nov 12, 2013 at 12:31 AM, Tony Lindgren <tony@atomide.com> wrote:
> >
> > It would be best to set it up as omap-ctrl.c driver under drivers
> > somewhere with few functions exported for DSS and MMC drivers.
>
> I create small dummy driver based on phy-omap-control which can be
> used but before sending patch (with more updates) I would like to get
> some feedback if my direction is correct.

Cool thanks. Yeah what you have can easily be combined with the patches
Balaji has posted to make HSMMC use drivers/mfd/syscon.c via regmap
for the SCM register access. Maybe take a look at the work in progress
patches in thread:

[PATCH v4 0/7] mmc: omap_hsmmc: pbias dt and cleanup

And also see my comments regarding using the SCM GENERAL register area
as base for the syscon.c driver. That should work for your driver too,
right? And then you can access the SYSCON1 register that way from your
consumer driver ;)

Regards,

Tony

^ permalink raw reply

* Re: [PATCH 1/3] arm: omap2: Export devconf1 bypass and acbias.
From: Belisko Marek @ 2013-12-10 22:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131111233152.GR15154@atomide.com>

Hi Tony,

On Tue, Nov 12, 2013 at 12:31 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Belisko Marek <marek.belisko@gmail.com> [131111 14:01]:
>> Hi Tony,
>>
>> On Mon, Nov 11, 2013 at 5:49 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > * Marek Belisko <marek@goldelico.com> [131014 14:11]:
>> >> devconf1 reg access is localized only in mach-omap2 and we need to export
>> >> updating of devconf1 from omapdss venc driver (bypass and acbias bits).
>> >> Add simple api call which update only necessary bits.
>> > ...
>> >
>> >> +void update_bypass_acbias(bool bypass, bool acbias)
>> >> +{
>> >> +#ifdef CONFIG_ARCH_OMAP3
>> >> +     int val = omap_ctrl_readl(OMAP343X_CONTROL_DEVCONF1);
>> >> +
>> >> +     if (bypass)
>> >> +             val |= OMAP2_TVOUTBYPASS;
>> >> +     else
>> >> +             val &= ~OMAP2_TVOUTBYPASS;
>> >> +
>> >> +     if (acbias)
>> >> +             val |= OMAP2_TVACEN;
>> >> +     else
>> >> +             val &= ~OMAP2_TVACEN;
>> >> +
>> >> +     omap_ctrl_writel(val, OMAP343X_CONTROL_DEVCONF1);
>> >> +#endif
>> >
>> > If this is truly a pinmux, you could already access this
>> > using pinctrl-single,bits device tree driver.
>> >
>> > But I guess that won't work yet, so it's best to set this
>> > up as a separate driver like we've done for the USB PHY
>> > registers.
>>
>> Can you please point me to that driver directly? I cannot see benefit
>> of new driver as as it will be only dummy driver
>> with export_symbol_gpl of upper function. Can it sit then in dss
>> directory or somewhere else? Thanks.
>
> You can take a look at drivers/usb/phy/phy-omap-control.c.
>
> Then there's a device tree using example for control_devconf0 in
> Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt.
>
> Looks like CONTROL_DEVCONF1 contains a bunch of misc registers,
> and arch/arm/mach-omap2/hsmmc.c seems to be tinkering with it too.
>
> It would be best to set it up as omap-ctrl.c driver under drivers
> somewhere with few functions exported for DSS and MMC drivers.
I create small dummy driver based on phy-omap-control which can be
used but before sending patch (with more updates) I would like to get
some feedback if my direction is correct.
diff --git a/drivers/misc/omap-ctrl.c b/drivers/misc/omap-ctrl.c
new file mode 100644
index 0000000..ca2e576
--- /dev/null
+++ b/drivers/misc/omap-ctrl.c
@@ -0,0 +1,84 @@
+/*
+ * omap-ctrl.c - The part of omap control module.
+ *
+ * Copyright (C) 2013 Goldelico GmbH
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Author: Marek Belisko <marek@goldelico.com>
+ *
+ * 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.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+
+static u32 __iomem *devconf1;
+
+void omap_ctrl_tvoutbypass(int on)
+{
+    int val = readl(devconf1);
+
+    if (on)
+       val |= BIT(18);
+    else
+       val &= ~BIT(18);
+
+    writel(val, devconf1);
+}
+EXPORT_SYMBOL_GPL(omap_ctrl_tvoutbypass);
+
+
+void omap_ctrl_acen(int on)
+{
+    int val = readl(devconf1);
+
+    if (on)
+       val |= BIT(11);
+    else
+       val &= ~BIT(11);
+
+    writel(val, devconf1);
+}
+EXPORT_SYMBOL_GPL(omap_ctrl_acen);
+
+
+static int omap_ctrl_probe(struct platform_device *pdev)
+{
+    struct resource *res;
+
+    res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+       "devconf1");
+    devconf1 = devm_ioremap_resource(
+       &pdev->dev, res);
+    if (IS_ERR(devconf1))
+       return PTR_ERR(devconf1);
+
+    return 0;
+}
+
+struct platform_driver omap_ctrl_driver = {
+    .probe = omap_ctrl_probe,
+    .driver = {
+    .name = "omap-ctrl",
+    .owner = THIS_MODULE,
+    },
+};
+
+static int __init omap_ctrl_init(void)
+{
+    return platform_driver_register(&omap_ctrl_driver);
+}
+subsys_initcall(omap_ctrl_init);
+
+static void __exit omap_ctrl_exit(void)
+{
+    platform_driver_unregister(&omap_ctrl_driver);
+}

I use that driver with platform device in board file with proper
resources and it works fine. It could be easily added also DT
bindings.
Also you mention somewhere in drivers (but I think misc isn't right
place for that). Any opinion on that? Thanks for feedback.

>
> The reason why it should be a separate driver is that the system
> control module can live a separate runtime PM life from the
> drivers using the CONTROL_DEVCONF register bits.
>
> Regards,
>
> Tony

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com

^ 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