* [PATCH 13/21] video: neofb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 6:57 UTC (permalink / raw)
To: linux-fbdev
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/neofb.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/video/neofb.c b/drivers/video/neofb.c
index 7ef079c..891b0bf 100644
--- a/drivers/video/neofb.c
+++ b/drivers/video/neofb.c
@@ -2146,12 +2146,6 @@ static void neofb_remove(struct pci_dev *dev)
fb_destroy_modedb(info->monspecs.modedb);
neo_unmap_mmio(info);
neo_free_fb_info(info);
-
- /*
- * Ensure that the driver data is no longer
- * valid.
- */
- pci_set_drvdata(dev, NULL);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 14/21] video: pm2fb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 6:58 UTC (permalink / raw)
To: linux-fbdev
From 9e2c37ee1d5c2145fac113c483c9e6dd408f10fe Mon Sep 17 00:00:00 2001
From: Jingoo Han <jg1.han@samsung.com>
Date: Thu, 12 Sep 2013 15:11:07 +0900
Subject: [PATCH 14/21] video: pm2fb: remove unnecessary pci_set_drvdata()
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/pm2fb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/pm2fb.c b/drivers/video/pm2fb.c
index 81354ee..45d9a3f 100644
--- a/drivers/video/pm2fb.c
+++ b/drivers/video/pm2fb.c
@@ -1744,7 +1744,6 @@ static void pm2fb_remove(struct pci_dev *pdev)
iounmap(par->v_regs);
release_mem_region(fix->mmio_start, fix->mmio_len);
- pci_set_drvdata(pdev, NULL);
fb_dealloc_cmap(&info->cmap);
kfree(info->pixmap.addr);
framebuffer_release(info);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 15/21] video: pm3fb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 6:58 UTC (permalink / raw)
To: linux-fbdev
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/pm3fb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/pm3fb.c b/drivers/video/pm3fb.c
index 7718faa..9c17474 100644
--- a/drivers/video/pm3fb.c
+++ b/drivers/video/pm3fb.c
@@ -1489,7 +1489,6 @@ static void pm3fb_remove(struct pci_dev *dev)
iounmap(par->v_regs);
release_mem_region(fix->mmio_start, fix->mmio_len);
- pci_set_drvdata(dev, NULL);
kfree(info->pixmap.addr);
framebuffer_release(info);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 16/21] video: s3fb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 6:59 UTC (permalink / raw)
To: linux-fbdev
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/s3fb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/s3fb.c b/drivers/video/s3fb.c
index 47ca86c..d158d2c 100644
--- a/drivers/video/s3fb.c
+++ b/drivers/video/s3fb.c
@@ -1431,7 +1431,6 @@ static void s3_pci_remove(struct pci_dev *dev)
pci_release_regions(dev);
/* pci_disable_device(dev); */
- pci_set_drvdata(dev, NULL);
framebuffer_release(info);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 17/21] video: savagefb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 6:59 UTC (permalink / raw)
To: linux-fbdev
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/savage/savagefb_driver.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/video/savage/savagefb_driver.c b/drivers/video/savage/savagefb_driver.c
index 741b239..4dbf45f 100644
--- a/drivers/video/savage/savagefb_driver.c
+++ b/drivers/video/savage/savagefb_driver.c
@@ -2362,12 +2362,6 @@ static void savagefb_remove(struct pci_dev *dev)
kfree(info->pixmap.addr);
pci_release_regions(dev);
framebuffer_release(info);
-
- /*
- * Ensure that the driver data is no longer
- * valid.
- */
- pci_set_drvdata(dev, NULL);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 18/21] video: sisfb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 7:00 UTC (permalink / raw)
To: linux-fbdev
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/sis/sis_main.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/video/sis/sis_main.c b/drivers/video/sis/sis_main.c
index 977e279..793b402 100644
--- a/drivers/video/sis/sis_main.c
+++ b/drivers/video/sis/sis_main.c
@@ -5994,7 +5994,6 @@ static int sisfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if(!ivideo->sisvga_enabled) {
if(pci_enable_device(pdev)) {
if(ivideo->nbridge) pci_dev_put(ivideo->nbridge);
- pci_set_drvdata(pdev, NULL);
framebuffer_release(sis_fb_info);
return -EIO;
}
@@ -6211,7 +6210,6 @@ error_3: vfree(ivideo->bios_abase);
pci_dev_put(ivideo->lpcdev);
if(ivideo->nbridge)
pci_dev_put(ivideo->nbridge);
- pci_set_drvdata(pdev, NULL);
if(!ivideo->sisvga_enabled)
pci_disable_device(pdev);
framebuffer_release(sis_fb_info);
@@ -6523,8 +6521,6 @@ static void sisfb_remove(struct pci_dev *pdev)
mtrr_del(ivideo->mtrr, ivideo->video_base, ivideo->video_size);
#endif
- pci_set_drvdata(pdev, NULL);
-
/* If device was disabled when starting, disable
* it when quitting.
*/
--
1.7.10.4
^ permalink raw reply related
* [PATCH 19/21] video: tdfxfb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 7:00 UTC (permalink / raw)
To: linux-fbdev
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/tdfxfb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/tdfxfb.c b/drivers/video/tdfxfb.c
index 64bc28b..f761fe3 100644
--- a/drivers/video/tdfxfb.c
+++ b/drivers/video/tdfxfb.c
@@ -1646,7 +1646,6 @@ static void tdfxfb_remove(struct pci_dev *pdev)
pci_resource_len(pdev, 1));
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
- pci_set_drvdata(pdev, NULL);
fb_dealloc_cmap(&info->cmap);
framebuffer_release(info);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 20/21] video: tridentfb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 7:01 UTC (permalink / raw)
To: linux-fbdev
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/tridentfb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/tridentfb.c b/drivers/video/tridentfb.c
index ab57d38..7ed9a22 100644
--- a/drivers/video/tridentfb.c
+++ b/drivers/video/tridentfb.c
@@ -1553,7 +1553,6 @@ static void trident_pci_remove(struct pci_dev *dev)
iounmap(info->screen_base);
release_mem_region(tridentfb_fix.smem_start, tridentfb_fix.smem_len);
release_mem_region(tridentfb_fix.mmio_start, tridentfb_fix.mmio_len);
- pci_set_drvdata(dev, NULL);
kfree(info->pixmap.addr);
fb_dealloc_cmap(&info->cmap);
framebuffer_release(info);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 21/21] video: vt8623fb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 7:01 UTC (permalink / raw)
To: linux-fbdev
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/vt8623fb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/vt8623fb.c b/drivers/video/vt8623fb.c
index e9557fa..6b424dd 100644
--- a/drivers/video/vt8623fb.c
+++ b/drivers/video/vt8623fb.c
@@ -829,7 +829,6 @@ static void vt8623_pci_remove(struct pci_dev *dev)
pci_release_regions(dev);
/* pci_disable_device(dev); */
- pci_set_drvdata(dev, NULL);
framebuffer_release(info);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 06/21] video: gx1fb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 7:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002001ceaf84$d5e9b450$81bd1cf0$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/geode/gx1fb_core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/geode/gx1fb_core.c b/drivers/video/geode/gx1fb_core.c
index ebbaada..7551a04 100644
--- a/drivers/video/geode/gx1fb_core.c
+++ b/drivers/video/geode/gx1fb_core.c
@@ -399,7 +399,6 @@ static void gx1fb_remove(struct pci_dev *pdev)
release_mem_region(gx1_gx_base() + 0x8300, 0x100);
fb_dealloc_cmap(&info->cmap);
- pci_set_drvdata(pdev, NULL);
framebuffer_release(info);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 14/21] video: pm2fb: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-12 7:28 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002801ceaf85$71f3d740$55db85c0$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/pm2fb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/pm2fb.c b/drivers/video/pm2fb.c
index 81354ee..45d9a3f 100644
--- a/drivers/video/pm2fb.c
+++ b/drivers/video/pm2fb.c
@@ -1744,7 +1744,6 @@ static void pm2fb_remove(struct pci_dev *pdev)
iounmap(par->v_regs);
release_mem_region(fix->mmio_start, fix->mmio_len);
- pci_set_drvdata(pdev, NULL);
fb_dealloc_cmap(&info->cmap);
kfree(info->pixmap.addr);
framebuffer_release(info);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 2/3] video: xilinxfb: Use devm_kzalloc instead of kzalloc
From: Jingoo Han @ 2013-09-12 10:42 UTC (permalink / raw)
To: 'Michal Simek', linux-kernel, monstr
Cc: 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, 'Jingoo Han'
In-Reply-To: <f9868e54718ba467691085ff78061a447eef3635.1378965270.git.michal.simek@xilinx.com>
On Thursday, September 12, 2013 2:55 PM, Michal Simek wrote:
>
> Simplify driver probe and release function.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/xilinxfb.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
^ permalink raw reply
* Re: [PATCH 3/3] video: xilinxfb: Simplify error path
From: Jingoo Han @ 2013-09-12 10:46 UTC (permalink / raw)
To: 'Michal Simek', linux-kernel, monstr
Cc: 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, 'Jingoo Han'
In-Reply-To: <940bbdfc5c67282ab461b9c82b55f18fc34c959d.1378965270.git.michal.simek@xilinx.com>
On Thursday, September 12, 2013 2:55 PM, Michal Simek wrote:
>
> devm_iounmap is called automatically that's why remove it from the code
> dev_set_drvdata(dev, NULL) is called by generic code
> after device_release or on probe failure.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/xilinxfb.c | 28 ++++++----------------------
> 1 file changed, 6 insertions(+), 22 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 1/2] video: ARM CLCD: Add DT support
From: Pawel Moll @ 2013-09-12 13:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5230AAEB.2030607@wwwdotorg.org>
On Wed, 2013-09-11 at 18:39 +0100, Stephen Warren wrote:
> > From the core's perspective it's just a bit of extra memory, you could,
> > for example, put a MTD ram disk on it. It seems to deserve a
> > representation in the tree then.
>
> Yes, that's a good point. Perhaps it is reasonable to represent the
> memory somewhere then.
>
> I don't see why this memory couldn't exist in the regular /memory node;
> it is after all (admittedly slow) RAM. Obviously you'd want to cover the
> region with a /memreserve/ to avoid it being used just like any other
> RAM.
This would be pretty tricky in my particular case... The sram chip lives
on a motherboard, which lives behind a static memory interface (Chip
Select + offset). And the memreserve can only take "top level" address,
which can be different depending on the test chip (SoC) or FPGA SMM
being used.
> Perhaps the CLCD could reference the memreserve then?
How do you mean reference the memreserve? It's not a node, I don't think you can get
a phandle to it...
> Alternatively, if you want to represent the region as a regular node
> rather than a /memory entry, I'd expect there to be a binding defining
> what that node was, and the node to at least have a compatible value as
> well. I'm not sure how you would indicate that the node should be MTD
> vs. a resource for CLCD though; perhaps you'd use a different compatible
> value depending on the intended use of the memory?
Yes, that's exactly what I have now:
psram@1,00000000 {
compatible = "arm,vexpress-psram", "mtd-ram";
reg = <1 0x00000000 0x02000000>;
bank-width = <4>;
};
vram@2,00000000 {
compatible = "arm,vexpress-vram";
reg = <2 0x00000000 0x00800000>;
};
which allows mtd to use the psram.
> >> I'm not sure what the benefit of making this a standalone node is; why
> >> not just put the base/size directly into the video-ram property in the
> >> CLCD node?
> >
> > This is certainly an option. I've considered this as well, but thought
> > that the above made more sense.
> >
> > Having said that, there is actually a use case, although a very
> > non-probable one, for having a direct number there... The interconnect
> > that CLCD is connected to could have different mapping than the
> > processor's one. In other word, the memory seen by the core at X, could
> > be accessed by the CLCD at Y. Or, in even more quirky situation, the
> > core may not have access to the memory at all, with the graphics being
> > generated only by GPU (or any other third party). Then the value would
> > mean "the address you have to use when generating AMBA read transactions
> > to be get some meaningful data" and would have to be known explicitly.
> >
> > I guess it won't be hard to convince me it's a better option ;-)
>
> That's true. Everything in DT is currently set up to describe the CPU's
> memory map. Either we need to add some more properties to describe the
> potentially different memory map for other bus masters, and/or we should
> represent the various buses in DT, and any driver doing DMA should ask
> each bus's driver in turn to translate the core-visible address to a bus
> address so we can eventually end up with the address needed by the bus
> masters.
>
> That is indeed complex, and could at least in many situations simple be
> short-circuited by putting the relevant base address in each other bus
> master's own node/property. Hopefully we wouldn't get bitten by how
> non-general that could be.
Ok, so I'll make it a arm,pl11x,specific property. Actually two
properties - one bit I missed was the dual STN panel case, where they
have separate frame buffers. Something along the lines of:
Optional:
- arm,pl11x,framebuffer-base: a pair of two values, address and size,
defining the framebuffer for the upper
(or the only one) panel
- arm,pl11x,lower-framebuffer-base: as above, for the lower STN panel,
when present
Being very hardware-specific, it covers all possible quirky scenarios,
at the same time does not exclude the option of using the CMA region
phandles, if a particular system needed it.
> > - max-memory-bandwidth: maximum bandwidth in bytes per second that the
> > cell's memory interface can handle
> >
> > and can be then used to calculate maximum bpp depending on the selected
> > mode.
>
> Yes, that's a better property definition.
Ok, I'll go that way then. Whether derived from the system design
characteristics or by "hands on experiments", seems hardware-y enough.
> >>> +- display-timings: standard display timings sub-node, see
> >>> + Documentation/devicetree/bindings/video/display-timing.txt
> >>
> >> Should that be in a "Required sub-nodes" section (I assume required not
> >> optional) rather than "Optional Properties"?
> >
> > Right, the whole "panel" section is kept separately in a hope that CDF
> > (or DRM or whatever ;-) generic display pipeline bindings will deprecate
> > it. So the display-timings is required when optional panel properties
> > are present. Maybe I should split them into a separate sub-node?
> > Something along these lines (including the bandwidth example):
>
> I would assume that TFT-vs-STN is a pretty direct representation of the
> HW (IO bus to panel in particular), and hence wouldn't be replaced by
> CDF? I would expect CDF to represent the more generic properties. But, I
> haven't been following CDF too closely, so perhaps that's not true.
I'm not expecting CDF *bindings* to carry this information, no. I'm
expecting CDF core to provide me with this data in runtime (here's the
panel; what kind of panel are you? what modes do you provide? etc.) and
this is what my initial experiments used. The CLCD node only had the
video-ram and bandwidth-like properties plus a phandle to the display.
> If you're expecting this binding to change if/when CDF appears, perhaps
> it'd be better to wait for CDF, or plan for a new compatible property at
> that time, or add some property indicating old-style configuration vs
> new-style configuration once CDF is supported?
I'm expecting the panel description inside the CLCD node to be
deprecated, not changed, by the mythical CDF. As to waiting for it...
well. It's been over a year since first proposal ("generic panel
framework" as was it called then ;-). And over 2 years since VE gained
"DT support except for CLCD, which should be ready soon" ;-) And than 2
or 3 new platforms had to keep auxdata only for CLCD's sake. So, sure -
we can wait :-)
Paweł
^ permalink raw reply
* Re: [PATCH v2 1/2] video: ARM CLCD: Add DT support
From: Stephen Warren @ 2013-09-12 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1378991033.13571.33.camel@hornet>
On 09/12/2013 07:03 AM, Pawel Moll wrote:
> On Wed, 2013-09-11 at 18:39 +0100, Stephen Warren wrote:
...
>> Perhaps the CLCD could reference the memreserve then?
>
> How do you mean reference the memreserve? It's not a node, I don't think you can get
> a phandle to it...
Perhaps it's an extension that hasn't actually been implemented yet. I
thought I saw some reference to it in some long-past discussions. It's
possible that this feature has been dropped in preference to the "CMA"
bindings for defining reserved memory regions?
...
> Ok, so I'll make it a arm,pl11x,specific property. Actually two
> properties - one bit I missed was the dual STN panel case, where they
> have separate frame buffers. Something along the lines of:
>
> Optional:
>
> - arm,pl11x,framebuffer-base: a pair of two values, address and size,
> defining the framebuffer for the upper
> (or the only one) panel
> - arm,pl11x,lower-framebuffer-base: as above, for the lower STN panel,
> when present
>
> Being very hardware-specific, it covers all possible quirky scenarios,
> at the same time does not exclude the option of using the CMA region
> phandles, if a particular system needed it.
What do "upper" and "lower" mean in this context? I assume this somehow
refers to placement of the displays on the board. If so, it sounds like
there should be some better terminology; something that's specific to
the CLCD controller itself, rather than the environment it's used in.
Aalthough it does sound like this is in an FPGA and hence the controller
is board-specific, but I don't see why it couldn't be re-used in some
other FPGA-based board with different panel layout. Would "A" and "B" or
"main" and "primary" or "first" and "second" work better?
^ permalink raw reply
* Re: [PATCH 1/2] video: ARM CLCD: Add DT support
From: Pawel Moll @ 2013-09-12 15:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5230DD1C.6060606@gmail.com>
On Wed, 2013-09-11 at 22:14 +0100, Sylwester Nawrocki wrote:
> Yes, I wasn't aware then that your "video RAM" is a separate chip attached
> to a distinct bus.
> My idea was to reuse memory node structure, including "reserved-memory"
> compatible value (note there are some fixups pending and those names are
> subject to change). Not sure about the property in CLCD device node
> containing phandle to the memory node (currently 'memory-region').
As I just told Steven, I'll just make it painfully low-level property
defining "address you have to generate to access base of the
framebuffer". Of course one can still go the CMA way, if such need
arises.
> > Ok, I see what you're saying. Yes, this could be done. No, I don't claim
> > to have enough expertise either (micrometers??? :-O ;-) The other thing
> > is that I don't really expect generic CDF bindings to cover such things.
> > They will (hopefully) only describe what's connected with what. And the
> > drivers should know how. Of course they may need the dimensions& alike
> > in the tree (of course having them standardised would help here), but
> > it's not a CDF job to provide those.
>
> Of course it's always easier to define couple of DT properties that would
> cover part of functionality of some specific device. But then such
> properties should be prefixed with vendor name AFAIU, since they are
> not approved as common ones and useful for wider set of devices.
This is a policy that changed many times already...
Anyway, I think I've got an idea how to render the problem custom panel
properties invalid. If so, I'll send another version of the patch.
> From the device tree perspective CDF is just a collection of (display
> related) devices and a complete set of DT properties will be needed to
> describe them. Certainly some share of device-specific properties should
> be expected. Links between (sub)devices can be already described in DT by
> the binding documented in video-interfaces.txt.
Whether to use the v4l scheme or not still seems to be a bone of
contention between the video and the DRM/KMS folk, so I wouldn't draw
any conclusions yet.
Paweł
^ permalink raw reply
* Re: [PATCH v2 1/2] video: ARM CLCD: Add DT support
From: Pawel Moll @ 2013-09-12 15:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5231D5FA.4060508@wwwdotorg.org>
On Thu, 2013-09-12 at 15:55 +0100, Stephen Warren wrote:
> > Ok, so I'll make it a arm,pl11x,specific property. Actually two
> > properties - one bit I missed was the dual STN panel case, where they
> > have separate frame buffers. Something along the lines of:
> >
> > Optional:
> >
> > - arm,pl11x,framebuffer-base: a pair of two values, address and size,
> > defining the framebuffer for the upper
> > (or the only one) panel
> > - arm,pl11x,lower-framebuffer-base: as above, for the lower STN panel,
> > when present
> >
> > Being very hardware-specific, it covers all possible quirky scenarios,
> > at the same time does not exclude the option of using the CMA region
> > phandles, if a particular system needed it.
>
> What do "upper" and "lower" mean in this context? I assume this somehow
> refers to placement of the displays on the board. If so, it sounds like
> there should be some better terminology; something that's specific to
> the CLCD controller itself,
It is very CLCD specific indeed and nothing to do with the board :-)
http://arminfo.emea.arm.com/help/topic/com.arm.doc.ddi0293c/I899134.html
"Upper and Lower Panel Frame Base Address Registers".
> rather than the environment it's used in.
> Aalthough it does sound like this is in an FPGA and hence the controller
> is board-specific, but I don't see why it couldn't be re-used in some
> other FPGA-based board with different panel layout. Would "A" and "B" or
> "main" and "primary" or "first" and "second" work better?
Funnily enough I typed "secondary-framebuffer-base" first, but than
thought "no, let's match the spec" :-)
Paweł
^ permalink raw reply
* Re: [PATCH v2 1/2] video: ARM CLCD: Add DT support
From: Pawel Moll @ 2013-09-12 15:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1378999581.13571.57.camel@hornet>
On Thu, 2013-09-12 at 16:26 +0100, Pawel Moll wrote:
> http://arminfo.emea.arm.com/help/topic/com.arm.doc.ddi0293c/I899134.html
Sorry, should be:
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0161e/I899134.html
Paweł
^ permalink raw reply
* Re: [PATCH v2 1/2] video: ARM CLCD: Add DT support
From: Russell King - ARM Linux @ 2013-09-12 15:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5230AAEB.2030607@wwwdotorg.org>
On Wed, Sep 11, 2013 at 11:39:55AM -0600, Stephen Warren wrote:
> I don't know how constrained of a system CLCD is, but I do know that
> mode validation is a very complex process in some real-life graphics
> drivers.
Apart from maximum memory bus bandwidth, probably maximum output
bandwidth, maximum resolution (determined by what will fit in the
registers and RAM) there aren't that much constraints. The hardware
block is quite simple in that regard.
More the problem is to deal with two situations:
1. you have a particular panel connected to it which requires a certain
fixed timing regime.
2. you have the CLCD connected to a VGA or HDMI connector where the
timing is dependent on the connected display.
The former would be the subject of some kind of *common* DT
representation of the timing requirements of the connected panel.
For the latter, DT needs to specify how the EDID data is retrieved,
or if there is no mechanism for that, being able to provide a set
of allowable timing parameters (such as min/max vsync, min/max
hsync, max dotclock - in other words, the data which used to be
provided to X11 in the past.)
None of that is specific to CLCD though: it's the same problem as
the SA11x0 LCD controller or any other scanned video controller.
^ permalink raw reply
* Re: [PATCH v2 1/2] video: ARM CLCD: Add DT support
From: Russell King - ARM Linux @ 2013-09-12 15:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5231D5FA.4060508@wwwdotorg.org>
On Thu, Sep 12, 2013 at 08:55:54AM -0600, Stephen Warren wrote:
> On 09/12/2013 07:03 AM, Pawel Moll wrote:
> > Ok, so I'll make it a arm,pl11x,specific property. Actually two
> > properties - one bit I missed was the dual STN panel case, where they
> > have separate frame buffers. Something along the lines of:
> >
> > Optional:
> >
> > - arm,pl11x,framebuffer-base: a pair of two values, address and size,
> > defining the framebuffer for the upper
> > (or the only one) panel
> > - arm,pl11x,lower-framebuffer-base: as above, for the lower STN panel,
> > when present
> >
> > Being very hardware-specific, it covers all possible quirky scenarios,
> > at the same time does not exclude the option of using the CMA region
> > phandles, if a particular system needed it.
>
> What do "upper" and "lower" mean in this context? I assume this somehow
> refers to placement of the displays on the board.
CLCD is only one controller - it can only produce one image from one
framebuffer.
What this refers to is a requirement for dual STN displays - rather
than starting at one corner and working logically to the opposite
corner, these start at one corner and half way through the display at
the same time. So you scan out the upper half of the image at the
same time that you scan out the lower half of the image.
In order to give an image on the display where the conventional formula
works:
address = start + y * bytes_per_line + x * bits_per_pixel / 8
this means that the "upper" and "lower" start addresses are related to
the parameters of the display - you want the lower half to start at
the address where y = panel_y_res / 2 and x = 0.
You may also wish to change the start address (if you have enough RAM)
to support page flipping - where you switch between two framebuffers -
one which is currently being displayed while the other is being updated
by the CPU. You would use this to provide smooth video playback for
example.
So, DT has no business directly encoding this information - it should
be calculated from the information provided by the panel and the
timings required, and all that the CLCD driver should know is that
"we have this RAM which starts here, and extends for N bytes" - or
to use system memory instead.
^ permalink raw reply
* Re: [PATCH 1/2] video: ARM CLCD: Add DT support
From: Pawel Moll @ 2013-09-12 18:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1378999425.13571.54.camel@hornet>
On Thu, 2013-09-12 at 16:23 +0100, Pawel Moll wrote:
> Anyway, I think I've got an idea how to render the problem custom panel
> properties invalid. If so, I'll send another version of the patch.
So, instead of describing the panel I thought I'd go lower level and try
to describe the CLCD's signal functions. It gets down to something like
this:
8<----
- arm,pl11x,panel-data-pads: array of 24 cells, each of them describing
a function of one of the CLD pads,
starting from 0 up to 23; each pad can
be described by one of the following values:
- 0: reserved (not connected)
- 0x100-0x107: color upper STN panel data 0 to 7
- 0x110-0x117: color lower STN panel data 0 to 7
- 0x200-0x207: mono upper STN panel data 0 to 7
- 0x210-0x217: mono lower STN panel data 0 to 7
- 0x300-0x307: red component bit 0 to 7 of TFT panel data
- 0x310-0x317: green component bit 0 to 7 of TFT panel data
- 0x320-0x327: blue component bit 0 to 7 of TFT panel data
- 0x330: intensity bit of TFT panel data
Example set of values for standard
panel interfaces:
- PL110 single colour STN panel:
<0x107 0x106 0x105 0x104 0x103 0x102 0x101 0x100>,
<0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
- PL110 dual colour STN panel:
<0x107 0x106 0x105 0x104 0x103 0x102 0x101 0x100>,
<0x117 0x116 0x115 0x114 0x113 0x112 0x111 0x110>,
<0 0 0 0 0 0 0 0>;
- PL110 single mono 4-bit STN panel:
<0x203 0x202 0x201 0x200>,
<0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
- PL110 dual mono 4-bit STN panel:
<0x203 0x202 0x201 0x200>, <0 0 0 0>,
<0x213 0x212 0x211 0x210>,
<0 0 0 0 0 0 0 0 0 0 0 0>;
- PL110 single mono 8-bit STN panel:
<0x207 0x206 0x205 0x204 0x203 0x202 0x201 0x200>,
<0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
- PL110 dual mono 8-bit STN panel:
<0x207 0x206 0x205 0x204 0x203 0x202 0x201 0x200>,
<0x217 0x216 0x215 0x214 0x213 0x212 0x211 0x210>,
<0 0 0 0 0 0 0 0>;
- PL110 TFT (1:)5:5:5 panel:
<0x330 0x300 0x301 0x302 0x303 0x304>,
<0x330 0x310 0x311 0x312 0x313 0x314>,
<0x330 0x320 0x321 0x322 0x323 0x324>,
<0 0 0 0 0 0>
- PL110 and PL111 TFT RGB 888 panel:
<0x300 0x301 0x302 0x303 0x304 0x305 0x306 0x307>,
<0x310 0x311 0x312 0x313 0x314 0x315 0x316 0x317>,
<0x320 0x321 0x322 0x323 0x324 0x325 0x326 0x327>;
- PL111 single colour STN panel:
<0x100 0x101 0x102 0x103 0x104 0x105 0x106 0x107>,
<0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
- PL111 dual colour STN panel:
<0x100 0x101 0x102 0x103 0x104 0x105 0x106 0x107>,
<0x110 0x111 0x112 0x113 0x114 0x115 0x116 0x117>,
<0 0 0 0 0 0 0 0>;
- PL111 single mono 4-bit STN panel:
<0x200 0x201 0x202 0x203>,
<0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
- PL111 dual mono 4-bit STN panel:
<0x200 0x201 0x202 0x203>, <0 0 0 0>,
<0x210 0x211 0x212 0x213>,
<0 0 0 0 0 0 0 0 0 0 0 0>;
- PL111 single mono 8-bit STN panel:
<0x200 0x201 0x202 0x203 0x204 0x205 0x206 0x207>,
<0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;
- PL111 dual mono 8-bit STN panel:
<0x200 0x201 0x202 0x203 0x204 0x205 0x206 0x207>,
<0x210 0x211 0x212 0x213 0x214 0x215 0x216 0x217>,
<0 0 0 0 0 0 0 0>;
- PL111 TFT 4:4:4 panel:
<0 0 0 0>, <0x300 0x301 0x302 0x303>,
<0 0 0 0>, <0x310 0x311 0x312 0x313>,
<0 0 0 0>, <0x320 0x321 0x322 0x323>;
- PL111 TFT 5:6:5 panel:
<0 0 0>, <0x300 0x301 0x302 0x303 0x304>,
<0 0>, <0x310 0x311 0x312 0x313 0x314 0x315>,
<0 0 0>, <0x320 0x321 0x322 0x323 0x324>;
- PL111 TFT (1):5:5:5 panel:
<0 0 0>, <0x300 0x301 0x302 0x303 0x304>,
<0 0>, <0x330 0x310 0x311 0x312 0x313 0x314>,
<0 0 0>, <0x320 0x321 0x322 0x323 0x324>;
8<----
Of course the examples above could be #defined.
Such approach would also solve problem with bizarre cases like the
Versatile's (not Express ;-) muxer Russell mentioned (where there is a
de-facto custom wiring used). And no, I had a look at pinmux bindings
and I don't think they fit here at all...
Thoughts?
Paweł
^ permalink raw reply
* [PATCH] pwm-backlight: allow for non-increasing brightness levels
From: Mike Dunn @ 2013-09-12 18:35 UTC (permalink / raw)
To: thierry.reding
Cc: Mike Dunn, Richard Purdie, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Grant Likely,
Rob Herring, linux-pwm, linux-fbdev, linux-kernel, devicetree,
Robert Jarzmik, Marek Vasut
Currently the driver assumes that the values specified in the brightness-levels
device tree property increase as they are parsed from left to right. But boards
that invert the signal between the PWM output and the backlight will need to
specify decreasing brightness-levels. This patch removes the assumption that
the last element of the array is the max value, and instead searches the array
for the max value and uses that as the normalizing value when determining the
duty cycle.
Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---
drivers/video/backlight/pwm_bl.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 1fea627..d66aaa0 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -27,6 +27,7 @@ struct pwm_bl_data {
unsigned int period;
unsigned int lth_brightness;
unsigned int *levels;
+ unsigned int max_level;
int (*notify)(struct device *,
int brightness);
void (*notify_after)(struct device *,
@@ -57,7 +58,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
if (pb->levels) {
duty_cycle = pb->levels[brightness];
- max = pb->levels[max];
+ max = pb->levels[pb->max_level];
} else {
duty_cycle = brightness;
}
@@ -195,7 +196,15 @@ static int pwm_backlight_probe(struct platform_device *pdev)
}
if (data->levels) {
- max = data->levels[data->max_brightness];
+ int i, max_value = 0, max_idx = 0;
+ for (i = 0; i <= data->max_brightness; i++) {
+ if (data->levels[i] > max_value) {
+ max_value = data->levels[i];
+ max_idx = i;
+ }
+ }
+ pb->max_level = max_idx;
+ max = data->levels[max_idx];
pb->levels = data->levels;
} else
max = data->max_brightness;
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCH 1/2] video: ARM CLCD: Add DT support
From: Stephen Warren @ 2013-09-12 22:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1379009416.13571.68.camel@hornet>
On 09/12/2013 12:10 PM, Pawel Moll wrote:
> On Thu, 2013-09-12 at 16:23 +0100, Pawel Moll wrote:
>> Anyway, I think I've got an idea how to render the problem custom panel
>> properties invalid. If so, I'll send another version of the patch.
>
> So, instead of describing the panel I thought I'd go lower level and try
> to describe the CLCD's signal functions. It gets down to something like
> this:
>
> 8<----
> - arm,pl11x,panel-data-pads: array of 24 cells, each of them describing
> a function of one of the CLD pads,
> starting from 0 up to 23; each pad can
> be described by one of the following values:
> - 0: reserved (not connected)
> - 0x100-0x107: color upper STN panel data 0 to 7
...
> Such approach would also solve problem with bizarre cases like the
> Versatile's (not Express ;-) muxer Russell mentioned (where there is a
> de-facto custom wiring used). And no, I had a look at pinmux bindings
> and I don't think they fit here at all...
>
> Thoughts?
I think pinctrl would work just fine here. However, I suspect there's
little need for pinctrl. pinctrl is required when one module may impose
requirements on another module's muxable pins. I assume that the
configuration for pins on CLCD is only relevant for CLCD itself, so
there's no need to export an interface by which something else can
configure the pins. Now, if any of the pins could be e.g. "GPIO", then
this argument might not apply. Hence, I think pinctrl is only "possible"
here, not "required", and the approach above seems fine to me.
^ permalink raw reply
* Re: [PATCH 1/2] video: ARM CLCD: Add DT support
From: Russell King - ARM Linux @ 2013-09-12 22:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52323E01.2070902@wwwdotorg.org>
On Thu, Sep 12, 2013 at 04:19:45PM -0600, Stephen Warren wrote:
> On 09/12/2013 12:10 PM, Pawel Moll wrote:
> > Such approach would also solve problem with bizarre cases like the
> > Versatile's (not Express ;-) muxer Russell mentioned (where there is a
> > de-facto custom wiring used). And no, I had a look at pinmux bindings
> > and I don't think they fit here at all...
> >
> > Thoughts?
>
> I think pinctrl would work just fine here. However, I suspect there's
> little need for pinctrl. pinctrl is required when one module may impose
> requirements on another module's muxable pins. I assume that the
> configuration for pins on CLCD is only relevant for CLCD itself, so
> there's no need to export an interface by which something else can
> configure the pins.
Well, the hardware is much like this:
CLCD ---> Mux ---> output
and the mux is controlled by a register, and will manipulate the output
from the CLCD controller such that the framebuffer contents can be
RGB565, BGR565, XRGB1555, XBGR1555 or RGB888.
(Actually, the Versatile mux can only support one of the 1555 formats,
but I forget which it is.)
I'm not sure that its worth describing all these in DT explicitly, but
maybe encoding it as a special "versatile" version of the CLCD primecell
instead - especially as the external mux takes care of the colour
component order rather than the CLCD controller (and that requires a
bit of board specific code to deal with.)
^ permalink raw reply
* [PATCH V5 0/5] video: mmp: misc updates for mmp display driver
From: Zhou Zhu @ 2013-09-13 6:59 UTC (permalink / raw)
To: linux-fbdev
Resend this patch set, the V1 to V4 of which is sent by Jett Zhou.
This patch set contains some misc updates for mmp display controller:
1. Rbswap enhancement
2. Code refine.
3. Pitch calculation and video layer settings.
Changes:
V5/V4:
Just rebase.
V3/V2:
some updates according to comments.
Guoqing Li (2):
video: mmp: rb swap setting update for mmp display
video: mmp: optimize some register setting code
Jett.Zhou (1):
ARM: mmp: remove the legacy rbswap setting for ttc_dkb platform
Jing Xiang (2):
video: mmp: calculate pitch value when fb set win
video: mmp: add pitch info in mmp_win structure
arch/arm/mach-mmp/ttc_dkb.c | 4 +--
drivers/video/mmp/fb/mmpfb.c | 34 ++++++++++++-------
drivers/video/mmp/hw/mmp_ctrl.c | 71 ++++++++++++++++++++++-----------------
drivers/video/mmp/hw/mmp_ctrl.h | 5 +++
include/video/mmp_disp.h | 6 ++++
5 files changed, 75 insertions(+), 45 deletions(-)
--
1.7.9.5
^ 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