* 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 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 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
* [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 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
* 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 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 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: 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 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: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
* [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: 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
* 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] video: amba-clcd: Make CLCD driver available on more platforms
From: Mark Brown @ 2013-12-12 18:07 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1386868390-12231-1-git-send-email-broonie@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 453 bytes --]
On Thu, Dec 12, 2013 at 06:48:49PM +0100, Geert Uytterhoeven wrote:
> Currently ARM_AMBA exists on arm/arm64 only.
> Is there a patch in the pipeline to change that? Without such a change,
I don't know but it's plausible given how common AMBA is.
> this patch doesn't make any difference.
No, it does mean that ARM64 is included - previously the dependency was
only on ARM so it wasn't available on ARM64. COMPILE_TEST won't have an
impact though.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] video: amba-clcd: Make CLCD driver available on more platforms
From: Geert Uytterhoeven @ 2013-12-12 18:51 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1386868390-12231-1-git-send-email-broonie@kernel.org>
On Thu, Dec 12, 2013 at 7:07 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Dec 12, 2013 at 06:48:49PM +0100, Geert Uytterhoeven wrote:
>
>> Currently ARM_AMBA exists on arm/arm64 only.
>> Is there a patch in the pipeline to change that? Without such a change,
>
> I don't know but it's plausible given how common AMBA is.
>
>> this patch doesn't make any difference.
>
> No, it does mean that ARM64 is included - previously the dependency was
Sure.
> only on ARM so it wasn't available on ARM64. COMPILE_TEST won't have an
> impact though.
Sorry, I meant it doesn't have any impact on COMPILE_TEST.
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 1/3] arm: omap2: Export devconf1 bypass and acbias.
From: Tony Lindgren @ 2013-12-12 19:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAfyv35wKAK2PNjcXcNan9GC0x-MmH5k4xmzj_vKKhPeztHV-A@mail.gmail.com>
On Thu, Dec 12, 2013 at 09:31:02AM +0100, Belisko Marek wrote:
> 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?
Yeah something like that. Or since the sysconf1 register is shared, it might
be best to still provide specific functions to access it if we cannot map
just specific bits of it separately for drivers using regmap.
But anyways this should for most part remove the need for a custom driver(s)
unless the register is a regulator or a clock.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information
From: Tony Lindgren @ 2013-12-12 21:59 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Laurent Pinchart, Tony Lindgren, linux-omap, linux-fbdev,
devicetree, Archit Taneja, Darren Etheridge
In-Reply-To: <52A9760A.3000801@ti.com>
On Thu, Dec 12, 2013 at 10:38:34AM +0200, Tomi Valkeinen wrote:
> On 2013-12-12 01:44, Laurent Pinchart wrote:
>
> 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 they have separate hwmod entries, they should be considered separate
independent devices for sure.
To summarize, here are few reasons why they need to be treated as
separate devices:
1. The modules maybe clocked/powered/idled separately and can have their
own idle configuration so they can do the hardware based idling
separately.
2. Doing a readback after a write to one module will not flush the write
to the other modules on the (bus depending on the SoC version AFAIK).
That can lead to nasty bugs caused by the ordering.
3. If the devices are described in a different way in the .dts files
from the hwmod data, we will not have 1-to-1 mapping and will never
be able to replace ti,hwmods with just the compatible string.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 10/26] OMAPDSS: add of helpers
From: Laurent Pinchart @ 2013-12-13 2:37 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
Darren Etheridge, Tony Lindgren
In-Reply-To: <52A96A4E.1010006@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]
Hi Tomi,
On Thursday 12 December 2013 09:48:30 Tomi Valkeinen wrote:
> On 2013-12-12 01:19, Laurent Pinchart wrote:
> > 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.
My plan is to keep it simple. I'll take the V4L2 code and add helpers needed
by this patch series and by my Renesas KMS drivers. I'll then see whether
refactoring makes sense, and will post the result. We will then add new
helpers whenever needed on a case by case basis.
--
Regards,
Laurent Pinchart
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information
From: Laurent Pinchart @ 2013-12-13 3:24 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Tony Lindgren, linux-omap, linux-fbdev, devicetree, Archit Taneja,
Darren Etheridge
In-Reply-To: <52A9760A.3000801@ti.com>
[-- Attachment #1: Type: text/plain, Size: 7493 bytes --]
Hi Tomi,
On Thursday 12 December 2013 10:38:34 Tomi Valkeinen wrote:
> 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.
The separate address spaces are not by themselves a clear indication that the
submodules should be considered as separate devices, as the hardware might
just group registers related to submodules together.
The dedicated interrupts (for DSI and HDMI) and PRCM clocks (for VENC if I'm
not mistaken, and HDMI on the OMAP4) are a clearer sign.
> 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.
After a quick look at the documentation I would say so. I would be tempted to
consider RFBI as part of the DSS core, but that's less clear.
> 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.
I agree with that, except for DPI, SDI and possibly RFBI.
> 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.
Right. The real question is whether the DSI or HDMI IPs can be used in a
system without the DSS core. If not, it might make sense to just merge the
drivers into a single module (of course with a clear interface between the
different parts to avoid spaghetti code).
> > 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
Given that the DSS core has a set of registers not dedicated to any of the
submodules I believe it should be represented by a device. The omapdss driver
thus doesn't look virtual to me, it supports a real piece of hardware.
> add the drivers for DSS modules in proper order.
>
> But then, I feel that they are quite independent and probably should be
> separate devices.
Even if they're separate devices they could be instantiated by DSS core based
on DT nodes. I'm not sure whether that's the best idea, but it might be worth
thinking about it.
> 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.
If the PLL and PHY are solely controlled through registers part of the DSS
core register space, they would seem like part of the DSS core to me.
--
Regards,
Laurent Pinchart
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information
From: Laurent Pinchart @ 2013-12-13 3:27 UTC (permalink / raw)
To: Tony Lindgren
Cc: Tomi Valkeinen, linux-omap, linux-fbdev, devicetree,
Archit Taneja, Darren Etheridge
In-Reply-To: <20131212215913.GB29072@muru.com>
Hi Tony,
On Thursday 12 December 2013 21:59:13 Tony Lindgren wrote:
> On Thu, Dec 12, 2013 at 10:38:34AM +0200, Tomi Valkeinen wrote:
> > On 2013-12-12 01:44, Laurent Pinchart wrote:
> >
> > 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 they have separate hwmod entries, they should be considered separate
> independent devices for sure.
>
> To summarize, here are few reasons why they need to be treated as
> separate devices:
Are you talking generally here, or about the DSS modules in particular ?
> 1. The modules maybe clocked/powered/idled separately and can have their
> own idle configuration so they can do the hardware based idling
> separately.
I don't think this applies to the DSS modules.
> 2. Doing a readback after a write to one module will not flush the write
> to the other modules on the (bus depending on the SoC version AFAIK).
> That can lead to nasty bugs caused by the ordering.
How does separate devices solve this ?
> 3. If the devices are described in a different way in the .dts files
> from the hwmod data, we will not have 1-to-1 mapping and will never
> be able to replace ti,hwmods with just the compatible string.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Laurent Pinchart @ 2013-12-13 3:45 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: <52A979D8.3040604@ti.com>
[-- Attachment #1: Type: text/plain, Size: 5743 bytes --]
Hi Tomi,
On Thursday 12 December 2013 10:54:48 Tomi Valkeinen wrote:
> On 2013-12-12 02:39, Laurent Pinchart wrote:
> > 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.
That's an option as well, but I'm not sure I like the idea of mixing fbdev and
generic video in a single directory. We could use a subdirectory of
drivers/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).
Yes, I've seen that. I need to analyze this in detail, that's planned for when
I'll work on sharing the DT parsing code.
> 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.
How do you handle the situation where the two drivers (for each end of the
link) need to know the bus width for instance ?
> >> 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.
I'm afraid I can't give you any guarantee here. The bindings will be unstable
for some time, and we'll have to deal with that somehow.
> 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.
That's a distinction I've missed.
> 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.
The omapdss platform data structure contains the following fields
- get_context_loss_count: What is that used for ?
- num_devices, devices, default_device: What are those used for ?
- default_display_name: This should be easy to move to DT.
- dsi_enable_pads, dsi_disable_pads: Those don't seem to be used in mainline.
What's their purpose, and how are they implemented on platforms that make use
of them ? Is the pinmux API an option ?
- set_min_bus_tput: We have the same problem for the OMAP3 ISP, so a generic
solution is needed here.
- version: Given that we detect the DSS revision based on the SoC revision,
I'm tempted to either explicitly encode the DSS revision in the compatible
string, or let the DSS driver query the SoC revision somehow. The later is
considered as a layering violation, but let's face it, I can't see the DSS
being used on a non-OMAP platform.
--
Regards,
Laurent Pinchart
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Geert Uytterhoeven @ 2013-12-13 8:16 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tomi Valkeinen,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Fbdev development list,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Archit Taneja,
Darren Etheridge, Tony Lindgren, Stefan Roese, Sebastian Reichel,
Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <1703598.NbqXAMAFOI@avalon>
On Fri, Dec 13, 2013 at 4:45 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > 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.
>
> That's an option as well, but I'm not sure I like the idea of mixing fbdev and
> generic video in a single directory. We could use a subdirectory of
> drivers/video.
Or reshuffle the various graphics/video/fb/console directories (more
bikeshedding). With git it's not that painful.
Frame buffer devices ended up in drivers/video because at that time, graphics
cards were called video cards. Moving video only entered the picture later.
drivers/fb/ (currently most of drivers/video/)
drivers/console/ (currently drivers/video/console/)
...
Or should fb be under gpu?
What about drivers/media? Video and audio are multi-media, too...
Baah, bad idea... too much bikeshedding ;-)
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 05/26] ARM: OMAP2+: add omapdss_init_of()
From: Tomi Valkeinen @ 2013-12-13 8:40 UTC (permalink / raw)
To: Archit Taneja, Laurent Pinchart
Cc: linux-omap, linux-fbdev, devicetree, Darren Etheridge,
Tony Lindgren
In-Reply-To: <52AAC615.504@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2206 bytes --]
On 2013-12-13 10:32, Archit Taneja wrote:
> On Thursday 12 December 2013 01:00 PM, Tomi Valkeinen wrote:
>> 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.
>
> Even if we remove the platform data, what would be the right place to
> register the drm, fb and vout devices? We need to make sure their
> drivers are probed only after omapdss is initialised.
That's the same issue as we have already, so removing omapdss doesn't
affect that.
I don't think we have any good way to ensure those devices are not
probed before omapdss. We just have to manage the case that omapdss has
not been probed yet, by checking if omapdss is there and if not, return
EPROBE_DEFER.
>> 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.
>
> I have seen other OMAP drivers passing control module register info to
> their DT node. Instead of having a pinmux driver for a single register,
> we might want to consider just passing the control module register, and
> take care of the reg fields and masks in the OMAP DSI driver itself.
Yes, that's also one option. Quite ugly, though =).
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: Archit Taneja @ 2013-12-13 8:44 UTC (permalink / raw)
To: Tomi Valkeinen, Laurent Pinchart
Cc: linux-omap, linux-fbdev, devicetree, Darren Etheridge,
Tony Lindgren
In-Reply-To: <52A96603.5030709@ti.com>
On Thursday 12 December 2013 01:00 PM, Tomi Valkeinen wrote:
> 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.
Even if we remove the platform data, what would be the right place to
register the drm, fb and vout devices? We need to make sure their
drivers are probed only after omapdss is initialised.
>
> 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.
I have seen other OMAP drivers passing control module register info to
their DT node. Instead of having a pinmux driver for a single register,
we might want to consider just passing the control module register, and
take care of the reg fields and masks in the OMAP DSI driver itself.
The parsing of the DSI pins from DT, however, can be kept generic.
Archit
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox