* Re: [RFC PATCH] video: Use fb_sys_write rather than open-coding in drivers
From: Ryan Mallon @ 2014-02-12 5:02 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jean-Christophe PLAGNIOL-VILLARD, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <52FB1AB0.6090601@ti.com>
On 12/02/14 19:54, Tomi Valkeinen wrote:
> On 11/02/14 21:07, Ryan Mallon wrote:
>> On 12/02/14 03:06, Tomi Valkeinen wrote:
>>
>>> On 20/09/13 10:06, Ryan Mallon wrote:
>>>> Several video drivers open code the fb_write write function with code
>>>> which is very similar to fb_sys_write. Replace the open code versions
>>>> with calls to fb_sys_write. An fb_sync callback is added to each of
>>>> the drivers.
>>>>
>>>> Signed-off-by: Ryan Mallon <rmallon@gmail.com>
>>>> ---
>>>
>>> Doesn't this change the behavior so that fb_write does no longer update
>>> the display, but fb_sync does? I don't think fb_sync is even meant to
>>> update the display, it's meant to wait for an update to finish. Then
>>> again, I'm not sure about that, all I see in fb.h is "wait for blit
>>> idle, optional"
>>
>>
>> fb_write() in fbmem.c calls ->fb_sync() after ->fb_write(), and I've set
>> the fb_sync() for each of the drivers, so the behaviour should be
>> unchanged for writes.
>>
>> The fb_sync() function is also called by fb_read() and
>> fb_get_buffer_offset() (if FB_PIXMAP_SYNC flag is set). I don't know if
>> that will adversely affect behaviour.
>
> Well, by just looking at the function names the drivers' fb_syncs call,
> it sounds to me that with your patch fb_sync will update the LCD, i.e.
> send data to it. Doing that in fb_read sounds totally wrong.
Well, the alternative is to supply an fb_write() implementation for each
driver that calls fb_sys_write(), and then updates the display. The
fb_sync() additions can be removed. That would cut down the boiler-plate
code, and should keep the behaviour the same.
If you don't think it is worth the effort, then the patch can just be
dropped.
~Ryan
^ permalink raw reply
* Re: [RFC PATCH] video: Use fb_sys_write rather than open-coding in drivers
From: Tomi Valkeinen @ 2014-02-12 6:54 UTC (permalink / raw)
To: Ryan Mallon
Cc: Jean-Christophe PLAGNIOL-VILLARD, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <52FA7509.1060704@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]
On 11/02/14 21:07, Ryan Mallon wrote:
> On 12/02/14 03:06, Tomi Valkeinen wrote:
>
>> On 20/09/13 10:06, Ryan Mallon wrote:
>>> Several video drivers open code the fb_write write function with code
>>> which is very similar to fb_sys_write. Replace the open code versions
>>> with calls to fb_sys_write. An fb_sync callback is added to each of
>>> the drivers.
>>>
>>> Signed-off-by: Ryan Mallon <rmallon@gmail.com>
>>> ---
>>
>> Doesn't this change the behavior so that fb_write does no longer update
>> the display, but fb_sync does? I don't think fb_sync is even meant to
>> update the display, it's meant to wait for an update to finish. Then
>> again, I'm not sure about that, all I see in fb.h is "wait for blit
>> idle, optional"
>
>
> fb_write() in fbmem.c calls ->fb_sync() after ->fb_write(), and I've set
> the fb_sync() for each of the drivers, so the behaviour should be
> unchanged for writes.
>
> The fb_sync() function is also called by fb_read() and
> fb_get_buffer_offset() (if FB_PIXMAP_SYNC flag is set). I don't know if
> that will adversely affect behaviour.
Well, by just looking at the function names the drivers' fb_syncs call,
it sounds to me that with your patch fb_sync will update the LCD, i.e.
send data to it. Doing that in fb_read sounds totally wrong.
> Note that I haven't actually tested this code since I don't have any of
> the hardware. It was just something I noticed while digging through
> framebuffer driver code.
Ok. Well, I think it's safer to drop these, then, if it's not 100% clear
that there are no unwanted side effects.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] video: exynos: Remove OF dependency for Exynos DP
From: Sachin Kamat @ 2014-02-12 7:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1389933172-22991-1-git-send-email-sachin.kamat@linaro.org>
On 11 February 2014 19:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 11/02/14 14:01, Sachin Kamat wrote:
>> On 10 February 2014 17:48, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> On 17/01/14 06:32, Sachin Kamat wrote:
>>>> Exynos is now a DT only platform. Hence there is no need
>>>> for an explicit OF dependency. Remove it.
>>>
>>> But the driver still depends on OF, doesn't it? I don't think it's very
>>> good for the driver Kconfig to make presumptions about what ARCH_*
>>> depend on.
>>
>> Depending upon nested dependencies is redundant IMHO.
>
> Well, a driver should be independent of the underlying arch. In
> practice, we have ARCH dependencies, as many of the devices only exist
> on that arch. But I think the drivers should still be designed to be
> arch-independent, as far as possible (omapdss compiles fine on x86).
>
> If the driver depends on OF, it should depend on OF in the Kconfig, no
> matter if the arch also depends on OF.
>
> I don't really care if the EXYNOS_LCD_S6E8AX0 has OF dependency or not,
> but to me this just looks unneeded cleanup, cluttering git logs, and in
> my opinion it's even going to the wrong direction.
Your argument makes sense. Upon further experimentation I found that even the
Exynos video drivers are ARCH independent (i.e., they build on x86 too) and do
not need to depend on OF for compilation. So I believe, we can remove both these
dependencies. What is your opinion?
--
With warm regards,
Sachin
^ permalink raw reply
* Re: [PATCH 1/2] video: exynos: Remove OF dependency for Exynos DP
From: Tomi Valkeinen @ 2014-02-12 7:25 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1389933172-22991-1-git-send-email-sachin.kamat@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 2510 bytes --]
On 12/02/14 09:08, Sachin Kamat wrote:
> On 11 February 2014 19:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 11/02/14 14:01, Sachin Kamat wrote:
>>> On 10 February 2014 17:48, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>> On 17/01/14 06:32, Sachin Kamat wrote:
>>>>> Exynos is now a DT only platform. Hence there is no need
>>>>> for an explicit OF dependency. Remove it.
>>>>
>>>> But the driver still depends on OF, doesn't it? I don't think it's very
>>>> good for the driver Kconfig to make presumptions about what ARCH_*
>>>> depend on.
>>>
>>> Depending upon nested dependencies is redundant IMHO.
>>
>> Well, a driver should be independent of the underlying arch. In
>> practice, we have ARCH dependencies, as many of the devices only exist
>> on that arch. But I think the drivers should still be designed to be
>> arch-independent, as far as possible (omapdss compiles fine on x86).
>>
>> If the driver depends on OF, it should depend on OF in the Kconfig, no
>> matter if the arch also depends on OF.
>>
>> I don't really care if the EXYNOS_LCD_S6E8AX0 has OF dependency or not,
>> but to me this just looks unneeded cleanup, cluttering git logs, and in
>> my opinion it's even going to the wrong direction.
>
> Your argument makes sense. Upon further experimentation I found that even the
> Exynos video drivers are ARCH independent (i.e., they build on x86 too) and do
> not need to depend on OF for compilation. So I believe, we can remove both these
> dependencies. What is your opinion?
Indeed, the driver doesn't even seem to call any of_* funcs. Looking at
the commit f9b1e013f1c6723798b8f7f5b83297e2837aaef7 (video: exynos_dp:
remove non-DT support for Exynos Display Port), it kind of sounds to me
that the OF dependency was put there just to prevent non-DT use.
I'm fine with removing OF dependency, if the commit description is
updated to say that it can be removed as the driver doesn't actually
depend on OF at all.
As for the ARCH dependency, I think we should keep it. I once removed
ARCH_OMAP dependency from omapdss, but Linus wasn't impressed when his
kernel compilation started to ask him if he wants to enable OMAPDSS
this, OMAPDSS that =). So I think it's fine to keep ARCH dependencies in
cases where the driver is clearly used only on some architecture.
However, you can use COMPILE_TEST kconfig option if you want to compile
test on other archs. I.e.:
depends on ARCH_EXYNOS || COMPILE_TEST
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 01/28] Remove CPU_MMP3
From: Geert Uytterhoeven @ 2014-02-12 7:52 UTC (permalink / raw)
To: Paul Bolle
Cc: Greg Kroah-Hartman, Richard Weinberger, Felipe Balbi,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
open list:USB PHY LAYER, open list, open list:FRAMEBUFFER LAYER
In-Reply-To: <1392156072.14317.26.camel@x220>
Cc Yu Xu <yuxu@marvell.com>
On Tue, Feb 11, 2014 at 11:01 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Tue, 2014-02-11 at 13:30 -0800, Greg Kroah-Hartman wrote:
>> On Sun, Feb 09, 2014 at 07:47:39PM +0100, Richard Weinberger wrote:
>> > The symbol is an orphan, get rid of it.
>> >
>> > Signed-off-by: Richard Weinberger <richard@nod.at>
>> > Acked-by: Paul Bolle <pebolle@tiscali.nl>
>> >
>> > ---
>> > drivers/usb/phy/Kconfig | 1 -
>> > drivers/video/mmp/Kconfig | 2 +-
>> > drivers/video/mmp/hw/Kconfig | 2 +-
>> > 3 files changed, 2 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> > index 7d1451d..b9b1c52 100644
>> > --- a/drivers/usb/phy/Kconfig
>> > +++ b/drivers/usb/phy/Kconfig
>> > @@ -61,7 +61,6 @@ config KEYSTONE_USB_PHY
>> >
>> > config MV_U3D_PHY
>> > bool "Marvell USB 3.0 PHY controller Driver"
>> > - depends on CPU_MMP3
>> > select USB_PHY
>> > help
>> > Enable this to support Marvell USB 3.0 phy controller for Marvell
>>
>> Do this and the driver breaks the build so it needs to depend on
>> something:
>>
>> drivers/usb/phy/phy-mv-u3d-usb.c: In function ‘mv_u3d_phy_read’:
>> drivers/usb/phy/phy-mv-u3d-usb.c:42:2: error: implicit declaration of function ‘writel_relaxed’ [-Werror=implicit-function-declaration]
>
> This sort of proves that a driver that depends on an unknown Kconfig
> symbol gets no build coverage (and will bit rot).
>
>> Sorry, I can't take this as-is.
>
> My mistake, I should have spotted this when looking a Richard's patch.
>
> Some background (which I quickly dug up): config MV_U3D_PHY got added in
> v3.7 through commit a67e76ac904c ("usb: phy: mv_u3d: Add usb phy driver
> for mv_u3d"). It then depended on USB_MV_U3D. And that symbol depended
> on CPU_MMP3 at that time. Ie, MV_U3D_PHY was unbuildable when it was
> added.
>
> In commit 60630c2eabd4 ("usb: gadget: mv_u3d: drop ARCH dependency")
> MV_U3D_PHY was made depended directly on CPU_MMP3. That kept it
> unbuildable, of course.
>
> Richard, assuming you agree with the above short history of MV_U3D_PHY,
> would you mind sending a v2 that removes this unbuildable driver?
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: [RFC PATCH] video: Use fb_sys_write rather than open-coding in drivers
From: Tomi Valkeinen @ 2014-02-12 8:17 UTC (permalink / raw)
To: Ryan Mallon
Cc: Jean-Christophe PLAGNIOL-VILLARD, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <52FB005E.1000207@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]
On 12/02/14 07:02, Ryan Mallon wrote:
> Well, the alternative is to supply an fb_write() implementation for each
> driver that calls fb_sys_write(), and then updates the display. The
> fb_sync() additions can be removed. That would cut down the boiler-plate
> code, and should keep the behaviour the same.
>
> If you don't think it is worth the effort, then the patch can just be
> dropped.
I'd be very cautious about doing cleanups on drivers that you cannot
test. Small innocent looking changes can break them.
For example, your patch sets info->screen_size, which is not set
currently. Does the fbdev framework use screen_size somewhere
differently than smem_len? I don't know, but that could lead to small
difference in operation. However, in this case, fb_sys_write() actually
uses smem_len if screen_size is 0, so that change is not even needed.
ssd1307fb_write() looks a bit different than fb_sys_write. I don't know
if the differences could cause issues. The other ones look copy-pasted
from fb_sys_write (but I didn't look at them carefully), so perhaps
those could be cleaned up safely.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] video: exynos: Remove OF dependency for Exynos DP
From: Sachin Kamat @ 2014-02-12 8:41 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1389933172-22991-1-git-send-email-sachin.kamat@linaro.org>
On 12 February 2014 12:55, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 12/02/14 09:08, Sachin Kamat wrote:
>> On 11 February 2014 19:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> On 11/02/14 14:01, Sachin Kamat wrote:
>>>> On 10 February 2014 17:48, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>>> On 17/01/14 06:32, Sachin Kamat wrote:
>>>>>> Exynos is now a DT only platform. Hence there is no need
>>>>>> for an explicit OF dependency. Remove it.
>>>>>
>>>>> But the driver still depends on OF, doesn't it? I don't think it's very
>>>>> good for the driver Kconfig to make presumptions about what ARCH_*
>>>>> depend on.
>>>>
>>>> Depending upon nested dependencies is redundant IMHO.
>>>
>>> Well, a driver should be independent of the underlying arch. In
>>> practice, we have ARCH dependencies, as many of the devices only exist
>>> on that arch. But I think the drivers should still be designed to be
>>> arch-independent, as far as possible (omapdss compiles fine on x86).
>>>
>>> If the driver depends on OF, it should depend on OF in the Kconfig, no
>>> matter if the arch also depends on OF.
>>>
>>> I don't really care if the EXYNOS_LCD_S6E8AX0 has OF dependency or not,
>>> but to me this just looks unneeded cleanup, cluttering git logs, and in
>>> my opinion it's even going to the wrong direction.
>>
>> Your argument makes sense. Upon further experimentation I found that even the
>> Exynos video drivers are ARCH independent (i.e., they build on x86 too) and do
>> not need to depend on OF for compilation. So I believe, we can remove both these
>> dependencies. What is your opinion?
>
> Indeed, the driver doesn't even seem to call any of_* funcs. Looking at
> the commit f9b1e013f1c6723798b8f7f5b83297e2837aaef7 (video: exynos_dp:
> remove non-DT support for Exynos Display Port), it kind of sounds to me
> that the OF dependency was put there just to prevent non-DT use.
>
> I'm fine with removing OF dependency, if the commit description is
> updated to say that it can be removed as the driver doesn't actually
> depend on OF at all.
>
> As for the ARCH dependency, I think we should keep it. I once removed
> ARCH_OMAP dependency from omapdss, but Linus wasn't impressed when his
> kernel compilation started to ask him if he wants to enable OMAPDSS
> this, OMAPDSS that =). So I think it's fine to keep ARCH dependencies in
> cases where the driver is clearly used only on some architecture.
Yes, I remember that :)
>
> However, you can use COMPILE_TEST kconfig option if you want to compile
> test on other archs. I.e.:
>
> depends on ARCH_EXYNOS || COMPILE_TEST
For now I will update the commit description and re-send the patch.
Thanks for your
comments Tomi.
--
With warm regards,
Sachin
^ permalink raw reply
* [PATCH Resend] video: exynos: Remove OF dependency for Exynos DP
From: Sachin Kamat @ 2014-02-12 8:45 UTC (permalink / raw)
To: linux-fbdev
OF dependency can be removed as the driver does not actually
depend on it at all.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Acked-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
index 1129d0e9e640..976594d578a9 100644
--- a/drivers/video/exynos/Kconfig
+++ b/drivers/video/exynos/Kconfig
@@ -30,7 +30,7 @@ config EXYNOS_LCD_S6E8AX0
config EXYNOS_DP
bool "EXYNOS DP driver support"
- depends on OF && ARCH_EXYNOS
+ depends on ARCH_EXYNOS
default n
help
This enables support for DP device.
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/1] video: exynos: Fix S6E8AX0 LCD driver build error
From: Sachin Kamat @ 2014-02-12 9:55 UTC (permalink / raw)
To: linux-fbdev
Enable S6E8AX0 LCD driver only if LCD_CLASS_DEVICE is a built-in driver.
Else we get the following errors due to missing symbols:
drivers/built-in.o: In function `s6e8ax0_probe':
:(.text+0x51aec): undefined reference to `lcd_device_register'
:(.text+0x51c44): undefined reference to `lcd_device_unregister'
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/video/exynos/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
index 976594d578a9..eb6f2b059821 100644
--- a/drivers/video/exynos/Kconfig
+++ b/drivers/video/exynos/Kconfig
@@ -22,7 +22,8 @@ config EXYNOS_MIPI_DSI
config EXYNOS_LCD_S6E8AX0
bool "S6E8AX0 MIPI AMOLED LCD Driver"
- depends on (EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE && LCD_CLASS_DEVICE)
+ depends on EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE
+ depends on (LCD_CLASS_DEVICE = y)
default n
help
If you have an S6E8AX0 MIPI AMOLED LCD Panel, say Y to enable its
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 1/1] video: exynos: Fix S6E8AX0 LCD driver build error
From: Sachin Kamat @ 2014-02-12 9:56 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1392198583-1828-1-git-send-email-sachin.kamat@linaro.org>
On 12 February 2014 15:19, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Enable S6E8AX0 LCD driver only if LCD_CLASS_DEVICE is a built-in driver.
> Else we get the following errors due to missing symbols:
> drivers/built-in.o: In function `s6e8ax0_probe':
> :(.text+0x51aec): undefined reference to `lcd_device_register'
> :(.text+0x51c44): undefined reference to `lcd_device_unregister'
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> drivers/video/exynos/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
> index 976594d578a9..eb6f2b059821 100644
> --- a/drivers/video/exynos/Kconfig
> +++ b/drivers/video/exynos/Kconfig
> @@ -22,7 +22,8 @@ config EXYNOS_MIPI_DSI
>
> config EXYNOS_LCD_S6E8AX0
> bool "S6E8AX0 MIPI AMOLED LCD Driver"
> - depends on (EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE && LCD_CLASS_DEVICE)
> + depends on EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE
> + depends on (LCD_CLASS_DEVICE = y)
> default n
> help
> If you have an S6E8AX0 MIPI AMOLED LCD Panel, say Y to enable its
> --
> 1.7.9.5
>
Sorry, please ignore this.
--
With warm regards,
Sachin
^ permalink raw reply
* [PATCH 1/1] video: s6e8ax0: Use devm_* APIs
From: Sachin Kamat @ 2014-02-12 9:58 UTC (permalink / raw)
To: linux-fbdev
devm_* APIs make the cleanup paths simpler.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/video/exynos/s6e8ax0.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/video/exynos/s6e8ax0.c b/drivers/video/exynos/s6e8ax0.c
index ca2602413aa4..29e70ed3f154 100644
--- a/drivers/video/exynos/s6e8ax0.c
+++ b/drivers/video/exynos/s6e8ax0.c
@@ -794,19 +794,18 @@ static int s6e8ax0_probe(struct mipi_dsim_lcd_device *dsim_dev)
return ret;
}
- lcd->ld = lcd_device_register("s6e8ax0", lcd->dev, lcd,
+ lcd->ld = devm_lcd_device_register(lcd->dev, "s6e8ax0", lcd->dev, lcd,
&s6e8ax0_lcd_ops);
if (IS_ERR(lcd->ld)) {
dev_err(lcd->dev, "failed to register lcd ops.\n");
return PTR_ERR(lcd->ld);
}
- lcd->bd = backlight_device_register("s6e8ax0-bl", lcd->dev, lcd,
- &s6e8ax0_backlight_ops, NULL);
+ lcd->bd = devm_backlight_device_register(lcd->dev, "s6e8ax0-bl",
+ lcd->dev, lcd, &s6e8ax0_backlight_ops, NULL);
if (IS_ERR(lcd->bd)) {
dev_err(lcd->dev, "failed to register backlight ops.\n");
- ret = PTR_ERR(lcd->bd);
- goto err_backlight_register;
+ return PTR_ERR(lcd->bd);
}
lcd->bd->props.max_brightness = MAX_BRIGHTNESS;
@@ -834,10 +833,6 @@ static int s6e8ax0_probe(struct mipi_dsim_lcd_device *dsim_dev)
dev_dbg(lcd->dev, "probed s6e8ax0 panel driver.\n");
return 0;
-
-err_backlight_register:
- lcd_device_unregister(lcd->ld);
- return ret;
}
#ifdef CONFIG_PM
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2 4/4] video: mmp: add device tree support
From: Tomi Valkeinen @ 2014-02-12 10:11 UTC (permalink / raw)
To: Zhou Zhu
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jean-Christophe Plagniol-Villard, Haojian Zhuang, Sascha Hauer,
Jingoo Han, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Chao Xie, Guoqing Li
In-Reply-To: <52F998B7.7060203-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4349 bytes --]
On 11/02/14 05:27, Zhou Zhu wrote:
> On 02/10/2014 08:43 PM, Tomi Valkeinen wrote:
>> How is the panel linked to all this? The nodes above do not refer to the
>> panel.
> We are making panel refer to path, so when panel dev probe, it could
> register to related path.
> The reason we not link from path to panel is our customer sometimes
> asked us to use same image pack (include dts) for different panel types
> in product. We could only add all these panels in dts and detect panel
> dynamically when boot. So moving panel out and making path not link to
> panel but panel link to path would be more straight forward.
Ok.
>>
>>> + ...
>>> + marvell,path = <&path1>;
>>> + ...
>>> +};
>>
>> It's a bit difficult to say much about this, as I have no knowledge
>> about mmp.
>>
>> But I don't quite understand what the pxa988-fb is. Is that some kind of
>> virtual device, only used to set up fbdev side? And pxa988-disp is the
>> actual hardware device?
>>
>> If so, I don't really think pxa988-fb should exist in the DT data at
>> all, as it's not a hardware device.
> Yes, fb is a virtual device for fbdev side.
> In our platforms we may use more than one fb for different paths/output
> panel or multi overlays on same path for composition. So we need to
> configure fb settings like which path/overlay/dmafetch it connected to
> for one or more fbdev.
> Besides, some more configures like yres_virtual size or fixed fb mem
> reserved rather than allocated which depends on platforms are also
> needed although these features are not upstreamed yet.
> So we put fb as a dt node here for these configures.
Ok.
The device tree data should reflect the hardware. Not the software. So
when thinking what kind of DT data you should have, you should forget
the drivers, and just think on HW terms.
You might want to have a look at CDF (Common Display Framework)
discussions on linux-fbdev list, and the "[PATCHv3 00/41] OMAPDSS: DT
support v3" series I've posted (mostly the .dts parts).
It sounds to me that you'd benefit greatly from the CDF, and even if CDF
is not yet merged (and will probably still take time), I'd very much
recommend trying to create your DT data in such a manner that it would
be in the same direction as what is currently suggested for CDF (or in
the OMAPDSS series).
>> Why isn't there just one node for pxa988-disp, which would contain all
>> the above information?
> We have moved out fb/panel from path due to several reasons:
> 1. To simplify the node. If moved these nodes in, it might be:
> disp {
> ...
> path1 {
> ...
> panel-xxx {
> }
> panel-yyy {
> }
> ...
> fb0 {
> }
> fb1 {
> }
> }
> path2 {
> ...
> panel-zzz {
> }
> fb3 {
> }
> }
> }
> Also due to child node type might be different, we could even not
> directly check child of node. The code would be complex.
> 2. the path of node would be too long and not so common.
> e.g. the panel node in dts would be /soc/axi/disp/path1/panel-xxx, and
> in sysfs, node would be /sys/devices/platform/soc/axi/path1/panel-xxx.
> It would be complex and not compatible for platforms when our
> bootloader/user app doing some operations to these nodes.
> If we moved them out, we could move fb/panel out of soc directory so the
> node would be /panel-xxx or /fb1 and simpler and compatible.
I suggest to first think only about the DT data, and create it
correctly. After that you could think how to create compatibility code
in the driver side, so that the legacy sysfs paths are still usable.
The thing with DT data is that it's quite difficult to make big changes
to it later, without writing possibly complex compatibility
functionality. So in my opinion it's worth it to spend a good deal of
time thinking about good DT bindings from the start.
That said, if the driver and hardware in question is for some old SoC,
that's not going to be used on new boards, and the driver is not going
to be used in any future boards, it might be simpler to just make simple
bindings that work for the known set of boards and displays, and be done
with it.
I don't know if that's the case here or not.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] video: exynos: Fix S6E8AX0 LCD driver build error
From: Tomi Valkeinen @ 2014-02-12 11:26 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1392198583-1828-1-git-send-email-sachin.kamat@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]
On 12/02/14 11:55, Sachin Kamat wrote:
> On 12 February 2014 15:19, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> Enable S6E8AX0 LCD driver only if LCD_CLASS_DEVICE is a built-in driver.
>> Else we get the following errors due to missing symbols:
>> drivers/built-in.o: In function `s6e8ax0_probe':
>> :(.text+0x51aec): undefined reference to `lcd_device_register'
>> :(.text+0x51c44): undefined reference to `lcd_device_unregister'
>>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>> drivers/video/exynos/Kconfig | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
>> index 976594d578a9..eb6f2b059821 100644
>> --- a/drivers/video/exynos/Kconfig
>> +++ b/drivers/video/exynos/Kconfig
>> @@ -22,7 +22,8 @@ config EXYNOS_MIPI_DSI
>>
>> config EXYNOS_LCD_S6E8AX0
>> bool "S6E8AX0 MIPI AMOLED LCD Driver"
>> - depends on (EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE && LCD_CLASS_DEVICE)
>> + depends on EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE
>> + depends on (LCD_CLASS_DEVICE = y)
>> default n
>> help
>> If you have an S6E8AX0 MIPI AMOLED LCD Panel, say Y to enable its
>> --
>> 1.7.9.5
>>
>
> Sorry, please ignore this.
>
Hmm? Why is the fix not needed?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] video: exynos: Fix S6E8AX0 LCD driver build error
From: Tomi Valkeinen @ 2014-02-12 11:42 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1392198583-1828-1-git-send-email-sachin.kamat@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]
On 12/02/14 13:41, Sachin Kamat wrote:
> On 12 February 2014 16:56, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 12/02/14 11:55, Sachin Kamat wrote:
>>> On 12 February 2014 15:19, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>>> Enable S6E8AX0 LCD driver only if LCD_CLASS_DEVICE is a built-in driver.
>>>> Else we get the following errors due to missing symbols:
>>>> drivers/built-in.o: In function `s6e8ax0_probe':
>>>> :(.text+0x51aec): undefined reference to `lcd_device_register'
>>>> :(.text+0x51c44): undefined reference to `lcd_device_unregister'
>>>>
>>>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>>> ---
>>>> drivers/video/exynos/Kconfig | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
>>>> index 976594d578a9..eb6f2b059821 100644
>>>> --- a/drivers/video/exynos/Kconfig
>>>> +++ b/drivers/video/exynos/Kconfig
>>>> @@ -22,7 +22,8 @@ config EXYNOS_MIPI_DSI
>>>>
>>>> config EXYNOS_LCD_S6E8AX0
>>>> bool "S6E8AX0 MIPI AMOLED LCD Driver"
>>>> - depends on (EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE && LCD_CLASS_DEVICE)
>>>> + depends on EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE
>>>> + depends on (LCD_CLASS_DEVICE = y)
>>>> default n
>>>> help
>>>> If you have an S6E8AX0 MIPI AMOLED LCD Panel, say Y to enable its
>>>> --
>>>> 1.7.9.5
>>>>
>>>
>>> Sorry, please ignore this.
>>>
>>
>> Hmm? Why is the fix not needed?
>
> You said you have already applied this for 3.14-fixes. I accidentally
> sent it again.
Ah ok. I thought the patch itself can be ignored (removed from my branch).
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] video: exynos: Fix S6E8AX0 LCD driver build error
From: Tomi Valkeinen @ 2014-02-12 11:51 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1392198583-1828-1-git-send-email-sachin.kamat@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 376 bytes --]
On 12/02/14 13:44, Sachin Kamat wrote:
> Btw, is this [1] your current tree as it doesn't look updated since some time?
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git
>
Yes, but I haven't pushed any 3.15 or 3.14-fixes content yet. I've been
rather busy in the previous weeks. I'll try to get the current patches
pushed today.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] video: exynos: Fix S6E8AX0 LCD driver build error
From: Sachin Kamat @ 2014-02-12 11:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1392198583-1828-1-git-send-email-sachin.kamat@linaro.org>
On 12 February 2014 17:21, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 12/02/14 13:44, Sachin Kamat wrote:
>
>> Btw, is this [1] your current tree as it doesn't look updated since some time?
>>
>> [1] git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git
>>
>
> Yes, but I haven't pushed any 3.15 or 3.14-fixes content yet. I've been
> rather busy in the previous weeks. I'll try to get the current patches
> pushed today.
Sounds good. Thanks.
--
With warm regards,
Sachin
^ permalink raw reply
* Re: [PATCH 1/1] video: exynos: Fix S6E8AX0 LCD driver build error
From: Sachin Kamat @ 2014-02-12 11:53 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1392198583-1828-1-git-send-email-sachin.kamat@linaro.org>
On 12 February 2014 16:56, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 12/02/14 11:55, Sachin Kamat wrote:
>> On 12 February 2014 15:19, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>> Enable S6E8AX0 LCD driver only if LCD_CLASS_DEVICE is a built-in driver.
>>> Else we get the following errors due to missing symbols:
>>> drivers/built-in.o: In function `s6e8ax0_probe':
>>> :(.text+0x51aec): undefined reference to `lcd_device_register'
>>> :(.text+0x51c44): undefined reference to `lcd_device_unregister'
>>>
>>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>> ---
>>> drivers/video/exynos/Kconfig | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
>>> index 976594d578a9..eb6f2b059821 100644
>>> --- a/drivers/video/exynos/Kconfig
>>> +++ b/drivers/video/exynos/Kconfig
>>> @@ -22,7 +22,8 @@ config EXYNOS_MIPI_DSI
>>>
>>> config EXYNOS_LCD_S6E8AX0
>>> bool "S6E8AX0 MIPI AMOLED LCD Driver"
>>> - depends on (EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE && LCD_CLASS_DEVICE)
>>> + depends on EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE
>>> + depends on (LCD_CLASS_DEVICE = y)
>>> default n
>>> help
>>> If you have an S6E8AX0 MIPI AMOLED LCD Panel, say Y to enable its
>>> --
>>> 1.7.9.5
>>>
>>
>> Sorry, please ignore this.
>>
>
> Hmm? Why is the fix not needed?
You said you have already applied this for 3.14-fixes. I accidentally
sent it again.
--
With warm regards,
Sachin
^ permalink raw reply
* Re: [PATCH 1/1] video: exynos: Fix S6E8AX0 LCD driver build error
From: Sachin Kamat @ 2014-02-12 11:56 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1392198583-1828-1-git-send-email-sachin.kamat@linaro.org>
On 12 February 2014 17:12, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 12/02/14 13:41, Sachin Kamat wrote:
>> On 12 February 2014 16:56, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> On 12/02/14 11:55, Sachin Kamat wrote:
>>>> On 12 February 2014 15:19, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>>>> Enable S6E8AX0 LCD driver only if LCD_CLASS_DEVICE is a built-in driver.
>>>>> Else we get the following errors due to missing symbols:
>>>>> drivers/built-in.o: In function `s6e8ax0_probe':
>>>>> :(.text+0x51aec): undefined reference to `lcd_device_register'
>>>>> :(.text+0x51c44): undefined reference to `lcd_device_unregister'
>>>>>
>>>>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>>>> ---
>>>>> drivers/video/exynos/Kconfig | 3 ++-
>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
>>>>> index 976594d578a9..eb6f2b059821 100644
>>>>> --- a/drivers/video/exynos/Kconfig
>>>>> +++ b/drivers/video/exynos/Kconfig
>>>>> @@ -22,7 +22,8 @@ config EXYNOS_MIPI_DSI
>>>>>
>>>>> config EXYNOS_LCD_S6E8AX0
>>>>> bool "S6E8AX0 MIPI AMOLED LCD Driver"
>>>>> - depends on (EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE && LCD_CLASS_DEVICE)
>>>>> + depends on EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE
>>>>> + depends on (LCD_CLASS_DEVICE = y)
>>>>> default n
>>>>> help
>>>>> If you have an S6E8AX0 MIPI AMOLED LCD Panel, say Y to enable its
>>>>> --
>>>>> 1.7.9.5
>>>>>
>>>>
>>>> Sorry, please ignore this.
>>>>
>>>
>>> Hmm? Why is the fix not needed?
>>
>> You said you have already applied this for 3.14-fixes. I accidentally
>> sent it again.
>
> Ah ok. I thought the patch itself can be ignored (removed from my branch).
Sorry for causing the confusion and noise.
Btw, is this [1] your current tree as it doesn't look updated since some time?
[1] git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git
--
With warm regards,
Sachin
^ permalink raw reply
* Fixing the kernels backlight API
From: Hans de Goede @ 2014-02-12 15:12 UTC (permalink / raw)
To: xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jingoo Han,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA
Hi All,
Quick self intro: I've been a FOSS developer for 15+ years now and I've been
working for Red Hat for 5 years. Recently I've moved to the graphics team.
One of my first tasks in the graphics team is to make the xserver run without
root rights. I'm making good progress with this, having solved almost all issues
already.
The biggest remaining stumbling block is the backlight API, because opening the
sysfs files requires root rights. I'll very likely write a little helper for this
for now, but in the long run it would be good to have a better solution.
While discussion this in the graphics devroom at Fosdem, the general consensus
seemed to be that the current backlight API is in need of an overhaul anyways.
There are several issues with the current API:
-there is no reliable way to determine the relation between a backlight
control in sysfs and the display it controls the backlight off
-on many laptops we end up with multiple providers of backlight control
all battling over control of the same backlight controller through various
firmware interfaces
-and there is no way to do acl management on it because of sysfs usage
At Fosdem it was suggested to "simply" make the backlight a property of
the connector in drm/kms and let users control it that way. From an acl pov
this makes a ton of sense, if a user controls the other display-panel settings,
then he should be able to control the backlight too.
This also nicely solves the issue of userspace having to figure out which backlight
control to use for a certain output.
Last this makes it the kernels responsibility to figure out which firmware interface
(if any) to use and tie that to the connector, rather then just exporting all of
them, including conflicting ones, and just hoping that userspace will figure things out.
Note that wrt the last point, the kernel is the one which should have all the hardware
knowledge to do this properly, after all hardware abstraction is one of the tasks of
the kernel.
I realize moving this more into the kernel, and tying things into drm is in no means
easy, but it is about time we clean up this mess.
Note that although I'm kicking of this discussion, my focus within the graphics team is
mostly on input devices, so I'm hoping that someone else will pick things up once we've
a better idea of how we would like to solve this.
Regards,
Hans
^ permalink raw reply
* Re: Fixing the kernels backlight API
From: Dave Airlie @ 2014-02-12 20:14 UTC (permalink / raw)
To: Hans de Goede, Matthew Garrett
Cc: Linux Fbdev development list, Jingoo Han,
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org, dri-devel
In-Reply-To: <52FB8F45.9060006-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
> The biggest remaining stumbling block is the backlight API, because opening the
> sysfs files requires root rights. I'll very likely write a little helper for this
> for now, but in the long run it would be good to have a better solution.
>
> While discussion this in the graphics devroom at Fosdem, the general consensus
> seemed to be that the current backlight API is in need of an overhaul anyways.
>
> There are several issues with the current API:
> -there is no reliable way to determine the relation between a backlight
> control in sysfs and the display it controls the backlight off
> -on many laptops we end up with multiple providers of backlight control
> all battling over control of the same backlight controller through various
> firmware interfaces
> -and there is no way to do acl management on it because of sysfs usage
>
> At Fosdem it was suggested to "simply" make the backlight a property of
> the connector in drm/kms and let users control it that way. From an acl pov
> this makes a ton of sense, if a user controls the other display-panel settings,
> then he should be able to control the backlight too.
>
> This also nicely solves the issue of userspace having to figure out which backlight
> control to use for a certain output.
>
> Last this makes it the kernels responsibility to figure out which firmware interface
> (if any) to use and tie that to the connector, rather then just exporting all of
> them, including conflicting ones, and just hoping that userspace will figure things out.
>
> Note that wrt the last point, the kernel is the one which should have all the hardware
> knowledge to do this properly, after all hardware abstraction is one of the tasks of
> the kernel.
>
> I realize moving this more into the kernel, and tying things into drm is in no means
> easy, but it is about time we clean up this mess.
>
> Note that although I'm kicking of this discussion, my focus within the graphics team is
> mostly on input devices, so I'm hoping that someone else will pick things up once we've
> a better idea of how we would like to solve this.
>
I hate to respond with yeah no, but yeah no,
http://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&show_html=true&highlight_names=&date 14-02-04
http://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&show_html=true&highlight_names=&date 14-02-05
read down that until you see me and tagr talking, read it a few times,
and the follow on chat with dvdhrm.
The biggest problem with leaving the kernel to pick the correct one,
is the kernel simply never knows which is the
correct one, and you also have to deal with a load of problems like
runtime module deps between very misc modules
or using some kind of notifier mechanism that will turn into a locking
nightmare.
I don't mean to dissuade you from trying to "fix" this, but actually
getting a solution upstream is going to require a lot of messing
around.
Talk to Matthew Garrett, if you haven't talked to him, then you
haven't talked to anyone who understands backlights.
Dave.
^ permalink raw reply
* Re: Fixing the kernels backlight API
From: Ville Syrjälä @ 2014-02-12 20:43 UTC (permalink / raw)
To: Dave Airlie
Cc: Linux Fbdev development list, Jingoo Han,
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org, dri-devel,
Hans de Goede
In-Reply-To: <CAPM=9tyYLuoTtW69-cPx-JSuar++D5WEtYRZr_-aP7uowZpVyA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Feb 13, 2014 at 06:14:04AM +1000, Dave Airlie wrote:
> >
> > The biggest remaining stumbling block is the backlight API, because opening the
> > sysfs files requires root rights. I'll very likely write a little helper for this
> > for now, but in the long run it would be good to have a better solution.
> >
> > While discussion this in the graphics devroom at Fosdem, the general consensus
> > seemed to be that the current backlight API is in need of an overhaul anyways.
> >
> > There are several issues with the current API:
> > -there is no reliable way to determine the relation between a backlight
> > control in sysfs and the display it controls the backlight off
> > -on many laptops we end up with multiple providers of backlight control
> > all battling over control of the same backlight controller through various
> > firmware interfaces
> > -and there is no way to do acl management on it because of sysfs usage
> >
> > At Fosdem it was suggested to "simply" make the backlight a property of
> > the connector in drm/kms and let users control it that way. From an acl pov
> > this makes a ton of sense, if a user controls the other display-panel settings,
> > then he should be able to control the backlight too.
> >
> > This also nicely solves the issue of userspace having to figure out which backlight
> > control to use for a certain output.
> >
> > Last this makes it the kernels responsibility to figure out which firmware interface
> > (if any) to use and tie that to the connector, rather then just exporting all of
> > them, including conflicting ones, and just hoping that userspace will figure things out.
> >
> > Note that wrt the last point, the kernel is the one which should have all the hardware
> > knowledge to do this properly, after all hardware abstraction is one of the tasks of
> > the kernel.
> >
> > I realize moving this more into the kernel, and tying things into drm is in no means
> > easy, but it is about time we clean up this mess.
> >
> > Note that although I'm kicking of this discussion, my focus within the graphics team is
> > mostly on input devices, so I'm hoping that someone else will pick things up once we've
> > a better idea of how we would like to solve this.
> >
>
> I hate to respond with yeah no, but yeah no,
>
> http://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&show_html=true&highlight_names=&date 14-02-04
> http://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&show_html=true&highlight_names=&date 14-02-05
>
> read down that until you see me and tagr talking, read it a few times,
> and the follow on chat with dvdhrm.
>
> The biggest problem with leaving the kernel to pick the correct one,
> is the kernel simply never knows which is the
> correct one,
That could be solved by still allowing userspace to change the
connection between the property and the actual backlight device.
With the prop approach + atomic modeset you could also do some
slightly fancier things like changing the backlight at the same
time as doing a modeset or just adjusting some other display
related properties.
> and you also have to deal with a load of problems like
> runtime module deps between very misc modules
> or using some kind of notifier mechanism that will turn into a locking
> nightmare.
This would still be an issue though.
I like the idea of the property, but I'm not volunteering to do any
of the work.
>
> I don't mean to dissuade you from trying to "fix" this, but actually
> getting a solution upstream is going to require a lot of messing
> around.
>
> Talk to Matthew Garrett, if you haven't talked to him, then you
> haven't talked to anyone who understands backlights.
>
> Dave.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
^ permalink raw reply
* [PATCH] fbdev: Make the switch from generic to native driver less alarming
From: Adam Jackson @ 2014-02-12 21:02 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1345223444-15852-1-git-send-email-ajax@redhat.com>
Calling this "conflicting" just makes people think there's a problem
when there's not.
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
drivers/video/fbmem.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 7309ac7..b6d5008 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1596,8 +1596,7 @@ static int do_remove_conflicting_framebuffers(struct apertures_struct *a,
(primary && gen_aper && gen_aper->count &&
gen_aper->ranges[0].base = VGA_FB_PHYS)) {
- printk(KERN_INFO "fb: conflicting fb hw usage "
- "%s vs %s - removing generic driver\n",
+ printk(KERN_INFO "fb: switching to %s from %s\n",
name, registered_fb[i]->fix.id);
ret = do_unregister_framebuffer(registered_fb[i]);
if (ret)
--
1.8.5.3
^ permalink raw reply related
* Re: Fixing the kernels backlight API
From: David Herrmann @ 2014-02-12 22:26 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Linux Fbdev development list, Jingoo Han, xorg-devel@lists.x.org,
dri-devel, Hans de Goede
In-Reply-To: <20140212204311.GH3891@intel.com>
Hi
On Wed, Feb 12, 2014 at 9:43 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Feb 13, 2014 at 06:14:04AM +1000, Dave Airlie wrote:
>> >
>> > The biggest remaining stumbling block is the backlight API, because opening the
>> > sysfs files requires root rights. I'll very likely write a little helper for this
>> > for now, but in the long run it would be good to have a better solution.
>> >
>> > While discussion this in the graphics devroom at Fosdem, the general consensus
>> > seemed to be that the current backlight API is in need of an overhaul anyways.
>> >
>> > There are several issues with the current API:
>> > -there is no reliable way to determine the relation between a backlight
>> > control in sysfs and the display it controls the backlight off
>> > -on many laptops we end up with multiple providers of backlight control
>> > all battling over control of the same backlight controller through various
>> > firmware interfaces
>> > -and there is no way to do acl management on it because of sysfs usage
>> >
>> > At Fosdem it was suggested to "simply" make the backlight a property of
>> > the connector in drm/kms and let users control it that way. From an acl pov
>> > this makes a ton of sense, if a user controls the other display-panel settings,
>> > then he should be able to control the backlight too.
>> >
>> > This also nicely solves the issue of userspace having to figure out which backlight
>> > control to use for a certain output.
>> >
>> > Last this makes it the kernels responsibility to figure out which firmware interface
>> > (if any) to use and tie that to the connector, rather then just exporting all of
>> > them, including conflicting ones, and just hoping that userspace will figure things out.
>> >
>> > Note that wrt the last point, the kernel is the one which should have all the hardware
>> > knowledge to do this properly, after all hardware abstraction is one of the tasks of
>> > the kernel.
>> >
>> > I realize moving this more into the kernel, and tying things into drm is in no means
>> > easy, but it is about time we clean up this mess.
>> >
>> > Note that although I'm kicking of this discussion, my focus within the graphics team is
>> > mostly on input devices, so I'm hoping that someone else will pick things up once we've
>> > a better idea of how we would like to solve this.
>> >
>>
>> I hate to respond with yeah no, but yeah no,
>>
>> http://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&show_html=true&highlight_names=&date 14-02-04
>> http://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&show_html=true&highlight_names=&date 14-02-05
>>
>> read down that until you see me and tagr talking, read it a few times,
>> and the follow on chat with dvdhrm.
>>
>> The biggest problem with leaving the kernel to pick the correct one,
>> is the kernel simply never knows which is the
>> correct one,
>
> That could be solved by still allowing userspace to change the
> connection between the property and the actual backlight device.
>
> With the prop approach + atomic modeset you could also do some
> slightly fancier things like changing the backlight at the same
> time as doing a modeset or just adjusting some other display
> related properties.
The "attach" stuff actually sounds doable, but who decides which one
to attach? You still need some user-space script during device-plug
for that.
But to be honest, the simplest way would be a "backlightd"
bus-activatable daemon. SetBacklight() then takes a DRM-connector and
brightness-value, which the daemon looks up in /sys and sets.. This
has the advantage that we can do any fancy matching in user-space. We
can provide quirks (maybe even via udev-hwdb) and other helpers for
weird setups.
Cheers
David
^ permalink raw reply
* Re: Fixing the kernels backlight API
From: Alexander E. Patrakov @ 2014-02-13 6:37 UTC (permalink / raw)
To: David Herrmann, Ville Syrjälä
Cc: Linux Fbdev development list, Jingoo Han,
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org, dri-devel,
Hans de Goede, Dave Airlie
In-Reply-To: <CANq1E4R=UVo5YVPUrdRFeobAE-qU6e-+GHZ2VJwwaa8zEO6g3Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
13.02.2014 04:26, David Herrmann wrote:
> The "attach" stuff actually sounds doable, but who decides which one
> to attach? You still need some user-space script during device-plug
> for that.
> But to be honest, the simplest way would be a "backlightd"
> bus-activatable daemon. SetBacklight() then takes a DRM-connector and
> brightness-value, which the daemon looks up in /sys and sets.. This
> has the advantage that we can do any fancy matching in user-space. We
> can provide quirks (maybe even via udev-hwdb) and other helpers for
> weird setups.
What would be done with Samsung monitors (like an old SyncMaster 770P)
that have a DVI connection, no physical buttons and have to be
controlled via DDC-CI? Currently, ddccontrol works (via /dev/i2c-*), but
only from root. I would like this use case to be covered in such a way
that it could work both in Xorg and in Wayland, and, if possible,
without races related to i2c usage from the kernel and from userspace.
--
Alexander E. Patrakov
^ permalink raw reply
* [PATCH 0/9] OMAP DSS DT bindings documentation
From: Tomi Valkeinen @ 2014-02-13 12:32 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA
Cc: Tomi Valkeinen
Hi,
Here is DT binding documentation for OMAP Display Subsystem. I've sent these
earlier as part of the whole DSS DT series, but I'm now sending them separately
to get comments for them.
These patches are essentially the same as what I already sent earlier. The only
difference is that I added clock information for omap3 and omap4 platforms.
Tomi
Tomi Valkeinen (9):
Doc/DT: Add OMAP DSS DT Bindings
Doc/DT: Add DT binding documentation for Analog TV Connector
Doc/DT: Add DT binding documentation for DVI Connector
Doc/DT: Add DT binding documentation for HDMI Connector
Doc/DT: Add DT binding documentation for MIPI DPI Panel
Doc/DT: Add DT binding documentation for MIPI DSI CM Panel
Doc/DT: Add DT binding documentation for Sony acx565akm panel
Doc/DT: Add DT binding documentation for TFP410 encoder
Doc/DT: Add DT binding documentation for tpd12s015 encoder
.../bindings/video/analog-tv-connector.txt | 23 +++
.../devicetree/bindings/video/dvi-connector.txt | 26 +++
.../devicetree/bindings/video/hdmi-connector.txt | 23 +++
.../devicetree/bindings/video/panel-dpi.txt | 43 +++++
.../devicetree/bindings/video/panel-dsi-cm.txt | 26 +++
.../devicetree/bindings/video/sony,acx565akm.txt | 28 +++
.../devicetree/bindings/video/ti,omap-dss.txt | 203 +++++++++++++++++++++
.../devicetree/bindings/video/ti,omap2-dss.txt | 54 ++++++
.../devicetree/bindings/video/ti,omap3-dss.txt | 83 +++++++++
.../devicetree/bindings/video/ti,omap4-dss.txt | 111 +++++++++++
.../devicetree/bindings/video/ti,tfp410.txt | 41 +++++
.../devicetree/bindings/video/ti,tpd12s015.txt | 44 +++++
.../devicetree/bindings/video/video-ports.txt | 22 +++
13 files changed, 727 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/analog-tv-connector.txt
create mode 100644 Documentation/devicetree/bindings/video/dvi-connector.txt
create mode 100644 Documentation/devicetree/bindings/video/hdmi-connector.txt
create mode 100644 Documentation/devicetree/bindings/video/panel-dpi.txt
create mode 100644 Documentation/devicetree/bindings/video/panel-dsi-cm.txt
create mode 100644 Documentation/devicetree/bindings/video/sony,acx565akm.txt
create mode 100644 Documentation/devicetree/bindings/video/ti,omap-dss.txt
create mode 100644 Documentation/devicetree/bindings/video/ti,omap2-dss.txt
create mode 100644 Documentation/devicetree/bindings/video/ti,omap3-dss.txt
create mode 100644 Documentation/devicetree/bindings/video/ti,omap4-dss.txt
create mode 100644 Documentation/devicetree/bindings/video/ti,tfp410.txt
create mode 100644 Documentation/devicetree/bindings/video/ti,tpd12s015.txt
create mode 100644 Documentation/devicetree/bindings/video/video-ports.txt
--
1.8.3.2
^ 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