Re: [PATCH v2 00/15] drm: Add a driver for FW-based Mali GPUs
On Wed, Aug 9, 2023 at 10:53 AM Boris Brezillon wrote: > > Hello, > > This is the second version of the kernel driver meant to support new Mali > GPUs which are delegating the scheduling to a firmware. [...] > > I tried to Cc anyone that was involved in any development of the code > I picked from panfrost, so they can acknowledge the GPL2 -> MIT+GPL2 > change. If I missed someone, please let me know. Regarding the relicensing, Linaro agrees to the relicensing of the parts they hold copyright on. Acked-by: Grant Likely -- Grant Likely CTO of Linaro, Ltd. grant.lik...@linaro.org
[BUG] blocked task after exynos_drm_init
On Tue, Nov 18, 2014 at 12:29 PM, Javier Martinez Canillas
wrote:
> [adding Kevin to cc list]
>
> Hello Inki,
>
> On Tue, Nov 18, 2014 at 11:52 AM, Inki Dae wrote:
>> On 2014ë
11ì 18ì¼ 19:42, Andrzej Hajda wrote:
>>> On 11/06/2014 10:06 AM, Krzysztof Kozlowski wrote:
Hi,
On last next (next-20141104, next-20141105) booting locks after
initializing Exynos DRM (Trats2 board):
[2.028283] [drm] Initialized drm 1.1.0 20060810
[ 240.505795] INFO: task swapper/0:1 blocked for more than 120 seconds.
[ 240.510825] Not tainted 3.18.0-rc3-next-20141105 #794
[ 240.516418] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
this message.
[ 240.524173] swapper/0 D c052534c 0 1 0 0x
[ 240.530527] [] (__schedule) from []
(schedule_preempt_disabled+0x14/0x20)
[ 240.539030] [] (schedule_preempt_disabled) from []
(mutex_lock_nested+0x1c4/0x464)
[ 240.548320] [] (mutex_lock_nested) from []
(__driver_attach+0x48/0x98)
[ 240.556562] [] (__driver_attach) from []
(bus_for_each_dev+0x54/0x88)
[ 240.564717] [] (bus_for_each_dev) from []
(bus_add_driver+0xe4/0x200)
[ 240.572876] [] (bus_add_driver) from []
(driver_register+0x78/0xf4)
[ 240.580864] [] (driver_register) from []
(exynos_drm_platform_probe+0x34/0x234)
[ 240.589890] [] (exynos_drm_platform_probe) from []
(platform_drv_probe+0x48/0xa4)
[ 240.599090] [] (platform_drv_probe) from []
(driver_probe_device+0x13c/0x37c)
[ 240.607940] [] (driver_probe_device) from []
(__driver_attach+0x94/0x98)
[ 240.616360] [] (__driver_attach) from []
(bus_for_each_dev+0x54/0x88)
[ 240.624517] [] (bus_for_each_dev) from []
(bus_add_driver+0xe4/0x200)
[ 240.632679] [] (bus_add_driver) from []
(driver_register+0x78/0xf4)
[ 240.640667] [] (driver_register) from []
(exynos_drm_init+0x70/0xa0)
[ 240.648739] [] (exynos_drm_init) from []
(do_one_initcall+0xac/0x1f0)
[ 240.656914] [] (do_one_initcall) from []
(kernel_init_freeable+0x10c/0x1d8)
[ 240.665591] [] (kernel_init_freeable) from []
(kernel_init+0x8/0xec)
[ 240.673661] [] (kernel_init) from []
(ret_from_fork+0x14/0x2c)
[ 240.681196] 3 locks held by swapper/0/1:
[ 240.685091] #0: (>mutex){..}, at: []
__driver_attach+0x48/0x98
[ 240.692732] #1: (>mutex){..}, at: []
__driver_attach+0x58/0x98
[ 240.700367] #2: (>mutex){..}, at: []
__driver_attach+0x48/0x98
>>>
>>>
>>> This is caused by patch moving platform devices to
>>> /sys/devices/platform[1]. Since this patch registering platform
>>> drivers/devices in probe of platform device causes deadlocks. I guess
>>> now all driver registration should be moved to exynos_drm_init and it
>>> seems better location for it IMHO.
>>
>> Thanks. It might be a chance that we could separate sub drivers of
>> Exynos drm into independent modules so that they can be called
>> independently because if we move them to exynos_drm_init then the
>> deferred probe wouldn't work correctly.
>>
>
> I don't understand why registering the platform drivers in the
> exynos_drm_init() will make deferred probing to not work correctly?
> AFAICT it does not matter where the driver is registered since if the
> driver probe function is called when the driver is attached and fails
> with -EPROBE_DEFER, it will be added to the deferred list and the
> probe function will be retried when other drivers are registered due
> devices being added (e.g: by OF when matching a compatible string). Or
> maybe I'm missing something here?
It's only by luck that it even worked before.
I think the problem is that exynos_drm_init() is registering a normal
(non-OF) platform device, so the parent will be /sys/devices/platform.
It immediately gets bound against exynos_drm_platform_driver which
calls the exynos drm_platform_probe() hook. The driver core obtains
device_lock() on the device *and on the device parent*.
Inside the probe hook, additional platform_drivers get registered.
Each time one does, it tries to bind against every platform device in
the system, which includes the ones created by OF. When it attempts to
bind, it obtains device_lock() on the device *and on the device
parent*.
Before the change to move of-generated platform devices into
/sys/devices/platform, the devices had different parents. Now both
devices have /sys/devices/platform as the parent, so yes they are
going to deadlock.
The real problem is registering drivers from within a probe hook. That
is completely wrong for the above deadlock reason. __driver_attach()
will deadlock. Those registrations must be pulled out of .probe().
Registering devices in .probe() is okay because __device_attach()
doesn't try to obtain device_lock() on the parent.
g.
>
> By the way, I tried moving the platform
[PATCH 0/9] Doc/DT: DT bindings for various display components
On Mon, 17 Mar 2014 15:55:27 +0200, Tomi Valkeinen wrote: > Hi Grant, > > Ping. > > Are you fine with me proceeding with the current V4L2 port/endpoint > bindings? Sorry, this thread didn't make it past my email filters. Yes, go ahead. g.
Re: [PATCH 22/51] DMA-API: amba: get rid of separate dma_mask
On Thu, 19 Sep 2013 22:47:01 +0100, Russell King rmk+ker...@arm.linux.org.uk wrote: AMBA Primecell devices always treat streaming and coherent DMA exactly the same, so there's no point in having the masks separated. Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk for the drivers/of/platform.c portion: Acked-by: Grant Likely grant.lik...@linaro.org g. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V3] i2c: move of helpers into the core
On Thu, 22 Aug 2013 18:00:14 +0200, Wolfram Sang w...@the-dreams.de wrote: I2C of helpers used to live in of_i2c.c but experience (from SPI) shows that it is much cleaner to have this in the core. This also removes a circular dependency between the helpers and the core, and so we can finally register child nodes in the core instead of doing this manually in each driver. So, fix the drivers and documentation, too. Acked-by: Rob Herring rob.herr...@calxeda.com Reviewed-by: Felipe Balbi ba...@ti.com Acked-by: Rafael J. Wysocki rafael.j.wyso...@intel.com Tested-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Wolfram Sang w...@the-dreams.de Acked-by: Grant Likely grant.lik...@linaro.org --- V2-V3: Was trying to be too smart by only fixing includes needed. Took a more general approach this time, converting of_i2c.h to i2c.h in case i2c.h was not already there. Otherwise remove it. Improved my build scripts and no build failures, no complaints from buildbot as well. Documentation/acpi/enumeration.txt |1 - arch/powerpc/platforms/44x/warp.c |1 - drivers/gpu/drm/tilcdc/tilcdc_slave.c |1 - drivers/gpu/drm/tilcdc/tilcdc_tfp410.c |1 - drivers/gpu/host1x/drm/output.c |2 +- drivers/i2c/busses/i2c-at91.c |3 - drivers/i2c/busses/i2c-cpm.c|6 -- drivers/i2c/busses/i2c-davinci.c|2 - drivers/i2c/busses/i2c-designware-platdrv.c |2 - drivers/i2c/busses/i2c-gpio.c |3 - drivers/i2c/busses/i2c-i801.c |2 - drivers/i2c/busses/i2c-ibm_iic.c|4 - drivers/i2c/busses/i2c-imx.c|3 - drivers/i2c/busses/i2c-mpc.c|2 - drivers/i2c/busses/i2c-mv64xxx.c|3 - drivers/i2c/busses/i2c-mxs.c|3 - drivers/i2c/busses/i2c-nomadik.c|3 - drivers/i2c/busses/i2c-ocores.c |3 - drivers/i2c/busses/i2c-octeon.c |3 - drivers/i2c/busses/i2c-omap.c |3 - drivers/i2c/busses/i2c-pnx.c|3 - drivers/i2c/busses/i2c-powermac.c |9 +- drivers/i2c/busses/i2c-pxa.c|2 - drivers/i2c/busses/i2c-s3c2410.c|2 - drivers/i2c/busses/i2c-sh_mobile.c |2 - drivers/i2c/busses/i2c-sirf.c |3 - drivers/i2c/busses/i2c-stu300.c |2 - drivers/i2c/busses/i2c-tegra.c |3 - drivers/i2c/busses/i2c-versatile.c |2 - drivers/i2c/busses/i2c-wmt.c|3 - drivers/i2c/busses/i2c-xiic.c |3 - drivers/i2c/i2c-core.c | 109 +- drivers/i2c/i2c-mux.c |3 - drivers/i2c/muxes/i2c-arb-gpio-challenge.c |1 - drivers/i2c/muxes/i2c-mux-gpio.c|1 - drivers/i2c/muxes/i2c-mux-pinctrl.c |1 - drivers/media/platform/exynos4-is/fimc-is-i2c.c |4 +- drivers/media/platform/exynos4-is/fimc-is.c |2 +- drivers/media/platform/exynos4-is/media-dev.c |1 - drivers/of/Kconfig |6 -- drivers/of/Makefile |1 - drivers/of/of_i2c.c | 114 --- drivers/staging/imx-drm/imx-tve.c |2 +- include/linux/i2c.h | 20 include/linux/of_i2c.h | 46 - sound/soc/fsl/imx-sgtl5000.c|2 +- sound/soc/fsl/imx-wm8962.c |2 +- 47 files changed, 138 insertions(+), 262 deletions(-) delete mode 100644 drivers/of/of_i2c.c delete mode 100644 include/linux/of_i2c.h diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt index d9be7a9..958266e 100644 --- a/Documentation/acpi/enumeration.txt +++ b/Documentation/acpi/enumeration.txt @@ -238,7 +238,6 @@ An I2C bus (controller) driver does: if (ret) /* handle error */ - of_i2c_register_devices(adapter); /* Enumerate the slave devices behind this bus via ACPI */ acpi_i2c_register_devices(adapter); diff --git a/arch/powerpc/platforms/44x/warp.c b/arch/powerpc/platforms/44x/warp.c index 4cfa499..534574a 100644 --- a/arch/powerpc/platforms/44x/warp.c +++ b/arch/powerpc/platforms/44x/warp.c @@ -16,7 +16,6 @@ #include linux/interrupt.h #include linux/delay.h #include linux/of_gpio.h -#include linux/of_i2c.h #include linux/slab.h #include linux/export.h diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c index dfffaf0..a19f657 100644
[PATCH V3] i2c: move of helpers into the core
On Thu, 22 Aug 2013 18:00:14 +0200, Wolfram Sang wrote: > I2C of helpers used to live in of_i2c.c but experience (from SPI) shows > that it is much cleaner to have this in the core. This also removes a > circular dependency between the helpers and the core, and so we can > finally register child nodes in the core instead of doing this manually > in each driver. So, fix the drivers and documentation, too. > > Acked-by: Rob Herring > Reviewed-by: Felipe Balbi > Acked-by: Rafael J. Wysocki > Tested-by: Sylwester Nawrocki > Signed-off-by: Wolfram Sang Acked-by: Grant Likely > --- > > V2->V3: Was trying to be too smart by only fixing includes needed. > Took a more general approach this time, converting of_i2c.h > to i2c.h in case i2c.h was not already there. Otherwise > remove it. Improved my build scripts and no build failures, > no complaints from buildbot as well. > > > Documentation/acpi/enumeration.txt |1 - > arch/powerpc/platforms/44x/warp.c |1 - > drivers/gpu/drm/tilcdc/tilcdc_slave.c |1 - > drivers/gpu/drm/tilcdc/tilcdc_tfp410.c |1 - > drivers/gpu/host1x/drm/output.c |2 +- > drivers/i2c/busses/i2c-at91.c |3 - > drivers/i2c/busses/i2c-cpm.c|6 -- > drivers/i2c/busses/i2c-davinci.c|2 - > drivers/i2c/busses/i2c-designware-platdrv.c |2 - > drivers/i2c/busses/i2c-gpio.c |3 - > drivers/i2c/busses/i2c-i801.c |2 - > drivers/i2c/busses/i2c-ibm_iic.c|4 - > drivers/i2c/busses/i2c-imx.c|3 - > drivers/i2c/busses/i2c-mpc.c|2 - > drivers/i2c/busses/i2c-mv64xxx.c|3 - > drivers/i2c/busses/i2c-mxs.c|3 - > drivers/i2c/busses/i2c-nomadik.c|3 - > drivers/i2c/busses/i2c-ocores.c |3 - > drivers/i2c/busses/i2c-octeon.c |3 - > drivers/i2c/busses/i2c-omap.c |3 - > drivers/i2c/busses/i2c-pnx.c|3 - > drivers/i2c/busses/i2c-powermac.c |9 +- > drivers/i2c/busses/i2c-pxa.c|2 - > drivers/i2c/busses/i2c-s3c2410.c|2 - > drivers/i2c/busses/i2c-sh_mobile.c |2 - > drivers/i2c/busses/i2c-sirf.c |3 - > drivers/i2c/busses/i2c-stu300.c |2 - > drivers/i2c/busses/i2c-tegra.c |3 - > drivers/i2c/busses/i2c-versatile.c |2 - > drivers/i2c/busses/i2c-wmt.c|3 - > drivers/i2c/busses/i2c-xiic.c |3 - > drivers/i2c/i2c-core.c | 109 +- > drivers/i2c/i2c-mux.c |3 - > drivers/i2c/muxes/i2c-arb-gpio-challenge.c |1 - > drivers/i2c/muxes/i2c-mux-gpio.c|1 - > drivers/i2c/muxes/i2c-mux-pinctrl.c |1 - > drivers/media/platform/exynos4-is/fimc-is-i2c.c |4 +- > drivers/media/platform/exynos4-is/fimc-is.c |2 +- > drivers/media/platform/exynos4-is/media-dev.c |1 - > drivers/of/Kconfig |6 -- > drivers/of/Makefile |1 - > drivers/of/of_i2c.c | 114 > --- > drivers/staging/imx-drm/imx-tve.c |2 +- > include/linux/i2c.h | 20 > include/linux/of_i2c.h | 46 - > sound/soc/fsl/imx-sgtl5000.c|2 +- > sound/soc/fsl/imx-wm8962.c |2 +- > 47 files changed, 138 insertions(+), 262 deletions(-) > delete mode 100644 drivers/of/of_i2c.c > delete mode 100644 include/linux/of_i2c.h > > diff --git a/Documentation/acpi/enumeration.txt > b/Documentation/acpi/enumeration.txt > index d9be7a9..958266e 100644 > --- a/Documentation/acpi/enumeration.txt > +++ b/Documentation/acpi/enumeration.txt > @@ -238,7 +238,6 @@ An I2C bus (controller) driver does: > if (ret) > /* handle error */ > > - of_i2c_register_devices(adapter); > /* Enumerate the slave devices behind this bus via ACPI */ > acpi_i2c_register_devices(adapter); > > diff --git a/arch/powerpc/platforms/44x/warp.c > b/arch/powerpc/platforms/44x/warp.c > index 4cfa499..534574a 100644 > --- a/arch/powerpc/platforms/44x/warp.c > +++ b/arch/powerpc/platforms/44x/warp.c > @@ -16,7 +16,6 @@ > #include > #include > #include > -#include >
Best practice device tree design for display subsystems/DRM
On Fri, Jul 5, 2013 at 11:07 AM, Sascha Hauer wrote: > Again the difference between supernodes and graphs is that the supernode > approach does not contain information about what components are needed > to do something useful with the device. You simply have to wait until > *all* components are present which may never happen if you don't drivers > for all components of the device. > With the graph instead you can start doing something once you find a > link between a source and a sink, no matter if other links are still > missing. I really think you're overstating your argument here. The whole point of a super node is that a *driver* can bind against it and a driver can be made intelligent enough to know which links are mandatory and which are optional (with the assumption that the data about which is which is encoded in the supernode). Graph vs. supernode vs some mixture of the two can all do exactly the same thing. What really matters is which approach best describes the hardware, and then the drivers can be designed based on that. > Another important point is that if you have a board with multiple i2c > encoder chips, how do you decide which one is connected to which LCDx > when all information you have is: "I need these x components", but have > no information how these components are connected to each other. > Fortunately I don't have hardware here which does something like this, > but it would also be possible to chain multiple encoder chips. This > could be described in a graph, but when all we have is a list of > components without connection information we would need board specific > code to handle the layout. It is still absolutely true that i2c devices must have a node below the i2c bus, same for spi, same for MMIO. That doesn't change. It really isn't an option to have the subservient devices directly under the supernode. On that point the supernode pretty much must have phandles to those device nodes. *However* there is absolutely nothing that says the subservient devices have to be bound to device drivers! It is perfectly fine for the supernode to go looking for a registered struct i2c_device (or whatever) and drive the device directly*. There are certainly cases where it wouldn't make sense to split a driver for what is effectively one device into two. But I digress. *You'll note that I said look for i2c_device here, and not device_node. The reason is that waiting for the i2c_device to appear gives a guarantee that the i2c bus is initialized. Looking for the device_node does not. g.
Best practice device tree design for display subsystems/DRM
On Wed, Jul 3, 2013 at 10:02 AM, Sascha Hauer wrote:
> On Wed, Jul 03, 2013 at 05:57:18PM +0900, Inki Dae wrote:
>> > video {
>> > /* Single video card w/ multiple lcd controllers */
>> > card0 {
>> > compatible = "marvell,armada-510-display";
>> > reg = <0 0x3f00 0x100>; /* video-mem hole */
>> > /* later: linux,video-memory-size = <0x100>; */
>> > marvell,video-devices = < >;
>> > };
>> >
>> > /* OR: Multiple video card w/ single lcd controllers */
>> > card0 {
>> > compatible = "marvell,armada-510-display";
>> > ...
>> > marvell,video-devices = <>;
>> > };
>> >
>> > card1 {
>> > compatible = "marvell,armada-510-display";
>> > ...
>> > marvell,video-devices = <>;
>> > };
>> > };
>>
>> Sorry but I'd like to say that this cannot be used commonly. Shouldn't you
>> really consider Linux framebuffer or other subsystems? The above dtsi file
>> is specific to DRM subsystem. And I think the dtsi file has no any
>> dependency on certain subsystem so board dtsi file should be considered for
>> all device drivers based on other subsystems: i.e., Linux framebuffer, DRM,
>> and so no. So I *strongly* object to it. All we have to do is to keep the
>> dtsi file as is, and to find other better way that can be used commonly in
>> DRM.
>
> +1 for not encoding the projected usecase of the graphics subsystem into
> the devicetree. Whether the two LCD controllers shall be used together
> or separately should not affect the devicetree. devicetree is about
> hardware description, not configuration.
It is however relevant to encode information about how devices are
related to each other. That could be an orthogonal binding though to
describe how displays are oriented relative to each other.
g.
Best practice device tree design for display subsystems/DRM
On Tue, Jul 2, 2013 at 9:25 PM, Russell King wrote: > On Tue, Jul 02, 2013 at 09:57:32PM +0200, Sebastian Hesselbarth wrote: >> I am against a super node which contains lcd and dcon/ire nodes. You can >> enable those devices on a per board basis. We add them to dove.dtsi but >> disable them by default (status = "disabled"). >> >> The DRM driver itself should get a video-card node outside of >> soc/internal-regs where you can put e.g. video memory hole (or video >> mem size if it will be taken from RAM later) >> >> About the unusual case, I guess we should try to get both lcd >> controllers into one DRM driver. Then support mirror or screen >> extension X already provides. For those applications where you want >> X on one lcd and some other totally different video stream - wait >> for someone to come up with a request or proposal. > > Well, all I can say then is that the onus is on those who want to treat > the components as separate devices to come up with some foolproof way > to solve this problem which doesn't involve making assumptions about > the way that devices are probed and doesn't end up creating artificial > restrictions on how the devices can be used - and doesn't end up burdening > the common case with lots of useless complexity that they don't need. > > It's _that_ case which needs to come up with a proposal about how to > handle it because you _can't_ handle it at the moment in any sane > manner which meets the criteria I've set out above, and at the moment > the best proposal by far to resolve that is the "super node" approach. > > There is _no_ way in the device model to combine several individual > devices together into one logical device safely when the subsystem > requires that there be a definite point where everything is known. > That applies even more so with -EPROBE_DEFER. With the presence of > such a thing, there is now no logical point where any code can say > definitively that the system has technically finished booting and all > resources are known. > > That's the problem - if you don't od the super-node approach, you end > up with lots of individual devices which you have to figure out some > way of combining, and coping with missing ones which might not be > available in the order you want them to be, etc. > > That's the advantage of the "super node" approach - it's a container > to tell you what's required in order to complete the creation of the > logical device, and you can parse the sub-nodes to locate the > information you need. Alternatively, you can have the same effect with a property or set of properties in the controller node that contains phandles to the required devices. That would provide the driver with the same information about which devices must be present. g.
Re: Best practice device tree design for display subsystems/DRM
On Tue, Jul 2, 2013 at 9:25 PM, Russell King r...@arm.linux.org.uk wrote: On Tue, Jul 02, 2013 at 09:57:32PM +0200, Sebastian Hesselbarth wrote: I am against a super node which contains lcd and dcon/ire nodes. You can enable those devices on a per board basis. We add them to dove.dtsi but disable them by default (status = disabled). The DRM driver itself should get a video-card node outside of soc/internal-regs where you can put e.g. video memory hole (or video mem size if it will be taken from RAM later) About the unusual case, I guess we should try to get both lcd controllers into one DRM driver. Then support mirror or screen extension X already provides. For those applications where you want X on one lcd and some other totally different video stream - wait for someone to come up with a request or proposal. Well, all I can say then is that the onus is on those who want to treat the components as separate devices to come up with some foolproof way to solve this problem which doesn't involve making assumptions about the way that devices are probed and doesn't end up creating artificial restrictions on how the devices can be used - and doesn't end up burdening the common case with lots of useless complexity that they don't need. It's _that_ case which needs to come up with a proposal about how to handle it because you _can't_ handle it at the moment in any sane manner which meets the criteria I've set out above, and at the moment the best proposal by far to resolve that is the super node approach. There is _no_ way in the device model to combine several individual devices together into one logical device safely when the subsystem requires that there be a definite point where everything is known. That applies even more so with -EPROBE_DEFER. With the presence of such a thing, there is now no logical point where any code can say definitively that the system has technically finished booting and all resources are known. That's the problem - if you don't od the super-node approach, you end up with lots of individual devices which you have to figure out some way of combining, and coping with missing ones which might not be available in the order you want them to be, etc. That's the advantage of the super node approach - it's a container to tell you what's required in order to complete the creation of the logical device, and you can parse the sub-nodes to locate the information you need. Alternatively, you can have the same effect with a property or set of properties in the controller node that contains phandles to the required devices. That would provide the driver with the same information about which devices must be present. g. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Best practice device tree design for display subsystems/DRM
On Wed, Jul 3, 2013 at 10:02 AM, Sascha Hauer s.ha...@pengutronix.de wrote:
On Wed, Jul 03, 2013 at 05:57:18PM +0900, Inki Dae wrote:
video {
/* Single video card w/ multiple lcd controllers */
card0 {
compatible = marvell,armada-510-display;
reg = 0 0x3f00 0x100; /* video-mem hole */
/* later: linux,video-memory-size = 0x100; */
marvell,video-devices = lcd0 lcd1 dcon;
};
/* OR: Multiple video card w/ single lcd controllers */
card0 {
compatible = marvell,armada-510-display;
...
marvell,video-devices = lcd0;
};
card1 {
compatible = marvell,armada-510-display;
...
marvell,video-devices = lcd1;
};
};
Sorry but I'd like to say that this cannot be used commonly. Shouldn't you
really consider Linux framebuffer or other subsystems? The above dtsi file
is specific to DRM subsystem. And I think the dtsi file has no any
dependency on certain subsystem so board dtsi file should be considered for
all device drivers based on other subsystems: i.e., Linux framebuffer, DRM,
and so no. So I *strongly* object to it. All we have to do is to keep the
dtsi file as is, and to find other better way that can be used commonly in
DRM.
+1 for not encoding the projected usecase of the graphics subsystem into
the devicetree. Whether the two LCD controllers shall be used together
or separately should not affect the devicetree. devicetree is about
hardware description, not configuration.
It is however relevant to encode information about how devices are
related to each other. That could be an orthogonal binding though to
describe how displays are oriented relative to each other.
g.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Best practice device tree design for display subsystems/DRM
On Fri, Jul 5, 2013 at 9:50 AM, Russell King r...@arm.linux.org.uk wrote: On Fri, Jul 05, 2013 at 09:37:34AM +0100, Grant Likely wrote: Alternatively, you can have the same effect with a property or set of properties in the controller node that contains phandles to the required devices. That would provide the driver with the same information about which devices must be present. How do you go from phandle to something-that-the-driver-for-that-device- has-setup? From what I can see, you can go from phandle to OF node, but no further. Correct, and that has historically been by design because it is possible for multiple struct devices to reference a single device_node. Any subsystem that needs to get a particular device has a lookup mechanism that searches the list of known devices and returns a match. example: of_mdio_find_bus() I'm guessing we'd need some kind of registry for sub-drivers with these structures to register their devices OF node plus shared data so that other drivers can find it. shared data might be a standardized operations struct or something similar to 'struct device_driver' but for componentised devices. If it is per-subsystem, the effort shouldn't be too high because it will be able to collect devices of the same type. It gets more complicated to design a generic componentised device abstraction (which I'm not opposed to, it's just going to be tricky and subtle). One big concern I have is differentiating between manditory and optional dependencies. Manditory is always easy to handle, but what about cases where the supernode (using phandles to other nodes) references other devices, but it is perfectly valid for the driver to complete probing before it becomes available? I may just be borrowing trouble here though. Supporting only mandatory dependencies in the first cut would still be a big step forward. One simple approach would be to add a depends on list to struct device and make the driver core check that all the dependant devices have drivers before probing. The risk is that would become a complicated set of reference counting and housekeeping. g. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Best practice device tree design for display subsystems/DRM
On Fri, Jul 5, 2013 at 10:34 AM, Sebastian Hesselbarth sebastian.hesselba...@gmail.com wrote: So for the discussion, I can see that there have been some voting for super-node, some for node-to-node linking. Although I initially proposed super-nodes, I can also happily live with node-to-node linking alone. Either someone can give an example where one of the approaches will not work (i.MX, exynos?), Grant or one of the DRM maintainers has a preference, or we are stuck at the decision. I tend to prefer a top-level super nodes with phandles to all of the components that compose the device when there is no clear one device that controls all the others. There is some precedence for that in other subsystems (leds, asoc, etc). Sound in particular has a lot of different bits and pieces that are interconnected with audio channels, gpios, and other things that get quite complicated, so it is convenient to have a single node that describes how they all fit together *and* allows for a platform to use a completely different device driver if required. node-to-node linking works well if there an absolute 'master' can be identified for the virtual device. ie. Ethernet MAC devices use a phy-device property to link to the phy it requires. In that case it is pretty clear that the Ethernet MAC is in charge and it uses the PHY. In either case it is absolutely required that the 'master' driver knows how to find and wait for all the subservient devices before probing can complete. I know that isn't a solid answer, but you know the problem space better than I. Take the above into account, make a decision and post a binding proposal for review. g. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Best practice device tree design for display subsystems/DRM
On Fri, Jul 5, 2013 at 11:07 AM, Sascha Hauer s.ha...@pengutronix.de wrote: Again the difference between supernodes and graphs is that the supernode approach does not contain information about what components are needed to do something useful with the device. You simply have to wait until *all* components are present which may never happen if you don't drivers for all components of the device. With the graph instead you can start doing something once you find a link between a source and a sink, no matter if other links are still missing. I really think you're overstating your argument here. The whole point of a super node is that a *driver* can bind against it and a driver can be made intelligent enough to know which links are mandatory and which are optional (with the assumption that the data about which is which is encoded in the supernode). Graph vs. supernode vs some mixture of the two can all do exactly the same thing. What really matters is which approach best describes the hardware, and then the drivers can be designed based on that. Another important point is that if you have a board with multiple i2c encoder chips, how do you decide which one is connected to which LCDx when all information you have is: I need these x components, but have no information how these components are connected to each other. Fortunately I don't have hardware here which does something like this, but it would also be possible to chain multiple encoder chips. This could be described in a graph, but when all we have is a list of components without connection information we would need board specific code to handle the layout. It is still absolutely true that i2c devices must have a node below the i2c bus, same for spi, same for MMIO. That doesn't change. It really isn't an option to have the subservient devices directly under the supernode. On that point the supernode pretty much must have phandles to those device nodes. *However* there is absolutely nothing that says the subservient devices have to be bound to device drivers! It is perfectly fine for the supernode to go looking for a registered struct i2c_device (or whatever) and drive the device directly*. There are certainly cases where it wouldn't make sense to split a driver for what is effectively one device into two. But I digress. *You'll note that I said look for i2c_device here, and not device_node. The reason is that waiting for the i2c_device to appear gives a guarantee that the i2c bus is initialized. Looking for the device_node does not. g. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] staging: drm/imx: use generic irqchip
On Fri, 21 Jun 2013 14:52:17 +0200, Philipp Zabel p.za...@pengutronix.de
wrote:
This depends on the patch genirq: Generic chip: Add linear irq domain
support
and removes the custom IPU irq_chip and irq_domain_ops. Instead, the generic
irq chip implementation is reused.
Signed-off-by: Philipp Zabel p.za...@pengutronix.de
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
drivers/staging/imx-drm/ipu-v3/ipu-common.c | 90
+
1 file changed, 26 insertions(+), 64 deletions(-)
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
index d629d6d..c135c66 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
@@ -986,53 +986,6 @@ static void ipu_err_irq_handler(unsigned int irq, struct
irq_desc *desc)
chained_irq_exit(chip, desc);
}
-static void ipu_ack_irq(struct irq_data *d)
-{
- struct ipu_soc *ipu = irq_data_get_irq_chip_data(d);
- irq_hw_number_t irq = d-hwirq;
-
- ipu_cm_write(ipu, 1 (irq % 32), IPU_INT_STAT(irq / 32));
-}
-
-static void ipu_unmask_irq(struct irq_data *d)
-{
- struct ipu_soc *ipu = irq_data_get_irq_chip_data(d);
- irq_hw_number_t irq = d-hwirq;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(ipu-lock, flags);
-
- reg = ipu_cm_read(ipu, IPU_INT_CTRL(irq / 32));
- reg |= 1 (irq % 32);
- ipu_cm_write(ipu, reg, IPU_INT_CTRL(irq / 32));
-
- spin_unlock_irqrestore(ipu-lock, flags);
-}
-
-static void ipu_mask_irq(struct irq_data *d)
-{
- struct ipu_soc *ipu = irq_data_get_irq_chip_data(d);
- irq_hw_number_t irq = d-hwirq;
- unsigned long flags;
- u32 reg;
-
- spin_lock_irqsave(ipu-lock, flags);
-
- reg = ipu_cm_read(ipu, IPU_INT_CTRL(irq / 32));
- reg = ~(1 (irq % 32));
- ipu_cm_write(ipu, reg, IPU_INT_CTRL(irq / 32));
-
- spin_unlock_irqrestore(ipu-lock, flags);
-}
-
-static struct irq_chip ipu_irq_chip = {
- .name = IPU,
- .irq_ack = ipu_ack_irq,
- .irq_mask = ipu_mask_irq,
- .irq_unmask = ipu_unmask_irq,
-};
-
int ipu_idmac_channel_irq(struct ipu_soc *ipu, struct ipuv3_channel *channel,
enum ipu_channel_irq irq_type)
{
@@ -1171,32 +1124,39 @@ err_register:
return ret;
}
-static int ipu_irq_map(struct irq_domain *h, unsigned int irq,
-irq_hw_number_t hw)
-{
- struct ipu_soc *ipu = h-host_data;
-
- irq_set_chip_and_handler(irq, ipu_irq_chip, handle_level_irq);
- set_irq_flags(irq, IRQF_VALID);
- irq_set_chip_data(irq, ipu);
-
- return 0;
-}
-
-const struct irq_domain_ops ipu_irq_domain_ops = {
- .map = ipu_irq_map,
- .xlate = irq_domain_xlate_onecell,
-};
static int ipu_irq_init(struct ipu_soc *ipu)
{
+ struct irq_chip_generic *gc;
+ struct irq_chip_type *ct;
+ int ret, i;
+
ipu-domain = irq_domain_add_linear(ipu-dev-of_node, IPU_NUM_IRQS,
- ipu_irq_domain_ops, ipu);
+ irq_generic_chip_ops, ipu);
if (!ipu-domain) {
dev_err(ipu-dev, failed to add irq domain\n);
return -ENODEV;
}
+ ret = irq_alloc_domain_generic_chips(ipu-domain, 32, 1, IPU,
+ handle_level_irq, 0, IRQF_VALID,
0);
+ if (ret 0) {
+ dev_err(ipu-dev, failed to alloc generic irq chips\n);
+ irq_domain_remove(ipu-domain);
+ return ret;
+ }
+
+ for (i = 0; i IPU_NUM_IRQS; i += 32) {
+ gc = irq_get_domain_generic_chip(ipu-domain, i);
+ gc-reg_base = ipu-cm_reg;
+ ct = gc-chip_types;
+ ct-chip.irq_ack = irq_gc_ack_set_bit;
+ ct-chip.irq_mask = irq_gc_mask_clr_bit;
+ ct-chip.irq_unmask = irq_gc_mask_set_bit;
+ ct-regs.ack = IPU_INT_STAT(i / 32);
+ ct-regs.mask = IPU_INT_CTRL(i / 32);
+ }
+
irq_set_chained_handler(ipu-irq_sync, ipu_irq_handler);
irq_set_handler_data(ipu-irq_sync, ipu);
irq_set_chained_handler(ipu-irq_err, ipu_err_irq_handler);
@@ -1214,6 +1174,8 @@ static void ipu_irq_exit(struct ipu_soc *ipu)
irq_set_chained_handler(ipu-irq_sync, NULL);
irq_set_handler_data(ipu-irq_sync, NULL);
+ /* TODO: remove irq_domain_generic_chips */
+
for (i = 0; i IPU_NUM_IRQS; i++) {
irq = irq_linear_revmap(ipu-domain, i);
if (irq)
--
1.8.3.1
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
email sent from notmuch.vim plugin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http
[PATCH 1/2] staging: drm/imx: use generic irqchip
On Fri, 21 Jun 2013 14:52:17 +0200, Philipp Zabel
wrote:
> This depends on the patch "genirq: Generic chip: Add linear irq domain
> support"
> and removes the custom IPU irq_chip and irq_domain_ops. Instead, the generic
> irq chip implementation is reused.
>
> Signed-off-by: Philipp Zabel
Acked-by: Grant Likely
> ---
> drivers/staging/imx-drm/ipu-v3/ipu-common.c | 90
> +
> 1 file changed, 26 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> index d629d6d..c135c66 100644
> --- a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> +++ b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> @@ -986,53 +986,6 @@ static void ipu_err_irq_handler(unsigned int irq, struct
> irq_desc *desc)
> chained_irq_exit(chip, desc);
> }
>
> -static void ipu_ack_irq(struct irq_data *d)
> -{
> - struct ipu_soc *ipu = irq_data_get_irq_chip_data(d);
> - irq_hw_number_t irq = d->hwirq;
> -
> - ipu_cm_write(ipu, 1 << (irq % 32), IPU_INT_STAT(irq / 32));
> -}
> -
> -static void ipu_unmask_irq(struct irq_data *d)
> -{
> - struct ipu_soc *ipu = irq_data_get_irq_chip_data(d);
> - irq_hw_number_t irq = d->hwirq;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(>lock, flags);
> -
> - reg = ipu_cm_read(ipu, IPU_INT_CTRL(irq / 32));
> - reg |= 1 << (irq % 32);
> - ipu_cm_write(ipu, reg, IPU_INT_CTRL(irq / 32));
> -
> - spin_unlock_irqrestore(>lock, flags);
> -}
> -
> -static void ipu_mask_irq(struct irq_data *d)
> -{
> - struct ipu_soc *ipu = irq_data_get_irq_chip_data(d);
> - irq_hw_number_t irq = d->hwirq;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(>lock, flags);
> -
> - reg = ipu_cm_read(ipu, IPU_INT_CTRL(irq / 32));
> - reg &= ~(1 << (irq % 32));
> - ipu_cm_write(ipu, reg, IPU_INT_CTRL(irq / 32));
> -
> - spin_unlock_irqrestore(>lock, flags);
> -}
> -
> -static struct irq_chip ipu_irq_chip = {
> - .name = "IPU",
> - .irq_ack = ipu_ack_irq,
> - .irq_mask = ipu_mask_irq,
> - .irq_unmask = ipu_unmask_irq,
> -};
> -
> int ipu_idmac_channel_irq(struct ipu_soc *ipu, struct ipuv3_channel *channel,
> enum ipu_channel_irq irq_type)
> {
> @@ -1171,32 +1124,39 @@ err_register:
> return ret;
> }
>
> -static int ipu_irq_map(struct irq_domain *h, unsigned int irq,
> -irq_hw_number_t hw)
> -{
> - struct ipu_soc *ipu = h->host_data;
> -
> - irq_set_chip_and_handler(irq, _irq_chip, handle_level_irq);
> - set_irq_flags(irq, IRQF_VALID);
> - irq_set_chip_data(irq, ipu);
> -
> - return 0;
> -}
> -
> -const struct irq_domain_ops ipu_irq_domain_ops = {
> - .map = ipu_irq_map,
> - .xlate = irq_domain_xlate_onecell,
> -};
>
> static int ipu_irq_init(struct ipu_soc *ipu)
> {
> + struct irq_chip_generic *gc;
> + struct irq_chip_type *ct;
> + int ret, i;
> +
> ipu->domain = irq_domain_add_linear(ipu->dev->of_node, IPU_NUM_IRQS,
> - _irq_domain_ops, ipu);
> + _generic_chip_ops, ipu);
> if (!ipu->domain) {
> dev_err(ipu->dev, "failed to add irq domain\n");
> return -ENODEV;
> }
>
> + ret = irq_alloc_domain_generic_chips(ipu->domain, 32, 1, "IPU",
> + handle_level_irq, 0, IRQF_VALID,
> 0);
> + if (ret < 0) {
> + dev_err(ipu->dev, "failed to alloc generic irq chips\n");
> + irq_domain_remove(ipu->domain);
> + return ret;
> + }
> +
> + for (i = 0; i < IPU_NUM_IRQS; i += 32) {
> + gc = irq_get_domain_generic_chip(ipu->domain, i);
> + gc->reg_base = ipu->cm_reg;
> + ct = gc->chip_types;
> + ct->chip.irq_ack = irq_gc_ack_set_bit;
> + ct->chip.irq_mask = irq_gc_mask_clr_bit;
> + ct->chip.irq_unmask = irq_gc_mask_set_bit;
> + ct->regs.ack = IPU_INT_STAT(i / 32);
> + ct->regs.mask = IPU_INT_CTRL(i / 32);
> + }
> +
> irq_set_chained_handler(ipu->irq_sync, ipu_irq_handler);
> irq_set_handler_data(ipu->irq_sync, ipu);
> irq_set_chained_handler(ipu->irq_err, ipu_err_irq_handler);
> @@ -1214,6 +1174,8 @@ static void ipu_irq_exit(struct ipu_soc *ipu)
> irq_set_chained_handler(ipu->irq_sync, NULL);
> irq_set_handler_data(ipu->irq_sync, NULL);
>
> + /* TODO: remove irq_domain_generic_chips */
> +
> for (i = 0; i < IPU_NUM_IRQS; i++) {
> irq = irq_linear_revmap(ipu->domain, i);
> if (irq)
> --
> 1.8.3.1
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
email sent from notmuch.vim plugin
[PATCH 9/9] ARM/dts: update device tree binding documentation for hdmi susbsystem
: Parent for mux mout_mixer. > > Example: > > @@ -15,4 +20,6 @@ Example: > compatible = "samsung,exynos5250-mixer"; > reg = <0x1445 0x1>; > interrupts = <0 94 0>; > + clocks = < 343>, < 136>; > + clock-names = "mixer", "sclk_hdmi"; > }; > -- > 1.7.10.4 > > ___ > devicetree-discuss mailing list > devicetree-discuss at lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd.
Re: [PATCH 9/9] ARM/dts: update device tree binding documentation for hdmi susbsystem
On Tue, 11 Jun 2013 19:41:31 +0530, Rahul Sharma rahul.sha...@samsung.com wrote: Update device tree binding documentation for hdmi subsystem with the clock information, phy property information and compatible strings for exynos5420. Signed-off-by: Rahul Sharma rahul.sha...@samsung.com Binding looks reasonable to me. I'll leave it to the video maintainers to say whether or not it is covers the right amount of configuration data. g. --- .../devicetree/bindings/video/exynos_hdmi.txt | 19 +++ .../devicetree/bindings/video/exynos_hdmiphy.txt| 10 -- .../devicetree/bindings/video/exynos_mixer.txt |7 +++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt b/Documentation/devicetree/bindings/video/exynos_hdmi.txt index 2ac01ca..e3c5853 100644 --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt @@ -4,6 +4,7 @@ Required properties: - compatible: value should be one among the following: 1) samsung,exynos4210-hdmi 2) samsung,exynos4212-hdmi + 3) samsung,exynos5420-hdmi - reg: physical base address of the hdmi and length of memory mapped region. - interrupts: interrupt number to the cpu. @@ -13,6 +14,20 @@ Required properties: c) pin function mode. d) optional flags and pull up/down. e) drive strength. +- clocks: list of clock IDs from SoC clock driver. + a) hdmi: It is required for gate operation on aclk_200_disp1 clock + which clocks the display1 block. + b) sclk_hdmi: It is required for gate operation on sclk_hdmi clock + which clocks hdmi IP. + c) sclk_pixel: Parent for mux mout_hdmi. + d) sclk_hdmiphy: Parent for mux mout_hdmi. + e) mout_hdmi: It is required by the driver to switch between the 2 + parents i.e. sclk_pixel and sclk_hdmiphy. If hdmiphy is stable + after configuration, parent is set to sclk_hdmiphy else + sclk_pixel. +- clock-names: aliases as per driver requirements for above clock IDs: + hdmi, sclk_hdmi, sclk_pixel, sclk_hdmiphy and mout_hdmi. +- phy: this property holds the phandle for hdmiphy node. Example: @@ -21,4 +36,8 @@ Example: reg = 0x1453 0x10; interrupts = 0 95 0; hpd-gpio = gpx3 7 0xf 1 3; + clocks = clock 344, clock 136, clock 137, + clock 157, clock 1024; + clock-names = hdmi, sclk_hdmi, sclk_pixel, + sclk_hdmiphy, mout_hdmi; }; diff --git a/Documentation/devicetree/bindings/video/exynos_hdmiphy.txt b/Documentation/devicetree/bindings/video/exynos_hdmiphy.txt index fb688a6..e2b12ed 100644 --- a/Documentation/devicetree/bindings/video/exynos_hdmiphy.txt +++ b/Documentation/devicetree/bindings/video/exynos_hdmiphy.txt @@ -1,8 +1,14 @@ Device-Tree bindings for hdmiphy driver Required properties: -- compatible: value should be samsung,exynos4210-hdmiphy. -- reg: I2C address of the hdmiphy device. +- compatible: value should be one among the following + 1) Samsung,exynos4210-hdmiphy. + 2) Samsung,exynos5420-hdmiphy. + +- reg: it holds the physical address infomration for the hdmiphy device. + If it is a i2c device, reg holds the I2C address of the phy. For + platform bus mapped phy, reg property holds physical address as + well as size of the register region. Example: diff --git a/Documentation/devicetree/bindings/video/exynos_mixer.txt b/Documentation/devicetree/bindings/video/exynos_mixer.txt index a8b063f..38e4e5c 100644 --- a/Documentation/devicetree/bindings/video/exynos_mixer.txt +++ b/Documentation/devicetree/bindings/video/exynos_mixer.txt @@ -4,10 +4,15 @@ Required properties: - compatible: value should be: 1) samsung,exynos4210-mixer 2) samsung,exynos5250-mixer + 3) samsung,exynos5420-mixer - reg: physical base address of the mixer and length of memory mapped region. - interrupts: interrupt number to the cpu. +- clocks: list of clock IDs from SoC clock driver. + a) mixer: It is required for gate operation on aclk_200_disp1 clock + which clocks the display1 block. + b) sclk_hdmi: Parent for mux mout_mixer. Example: @@ -15,4 +20,6 @@ Example: compatible = samsung,exynos5250-mixer; reg = 0x1445 0x1; interrupts = 0 94 0; + clocks = clock 343, clock 136; + clock-names = mixer, sclk_hdmi; }; -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-disc...@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd
[PATCH] ARM: dts: moving dt binding documents for video devices to common place
On Wed, 06 Feb 2013 09:51:39 -0500, Rahul Sharma wrote: > Binding Documents for drm-devices are placed in > Documentation/devicetree/bindings/drm/*. But these devices are common > for v4l framework, hence moved to a common place at > Documentation/devicetree/bindings/video/. 'exynos_' prefix is added to > associate them with exynos soc series. > > Signed-off-by: Rahul Sharma Applied, thanks. A tip however, if you use the "-M" flag when posting patches it will show the files as moved instead of delete the old copy and create a new. It makes my life easier when that is done. g.
Re: [PATCH] ARM: dts: moving dt binding documents for video devices to common place
On Wed, 06 Feb 2013 09:51:39 -0500, Rahul Sharma rahul.sha...@samsung.com wrote: Binding Documents for drm-devices are placed in Documentation/devicetree/bindings/drm/*. But these devices are common for v4l framework, hence moved to a common place at Documentation/devicetree/bindings/video/. 'exynos_' prefix is added to associate them with exynos soc series. Signed-off-by: Rahul Sharma rahul.sha...@samsung.com Applied, thanks. A tip however, if you use the -M flag when posting patches it will show the files as moved instead of delete the old copy and create a new. It makes my life easier when that is done. g. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv15 2/7] video: add display_timing and videomode
On Mon, 26 Nov 2012 16:39:58 +0100, Steffen Trumtrar wrote:
> On Mon, Nov 26, 2012 at 02:37:26PM +0200, Tomi Valkeinen wrote:
> > On 2012-11-26 11:07, Steffen Trumtrar wrote:
> >
> > > +/*
> > > + * Subsystem independent description of a videomode.
> > > + * Can be generated from struct display_timing.
> > > + */
> > > +struct videomode {
> > > + u32 pixelclock; /* pixelclock in Hz */
> >
> > I don't know if this is of any importance, but the linux clock framework
> > manages clock rates with unsigned long. Would it be better to use the
> > same type here?
> >
>
> Hm, I don't know. Anyone? u32 should be large enough for a pixelclock.
4GHz is a pretty large pixel clock. I have no idea how conceivable it is
that hardware will get to that speed. However, if it will ever be
larger, then you'll need to account for that in the DT binding so that
the pixel clock can be specified using 2 cells.
g.
Re: [PATCHv15 2/7] video: add display_timing and videomode
On Mon, 26 Nov 2012 16:39:58 +0100, Steffen Trumtrar
s.trumt...@pengutronix.de wrote:
On Mon, Nov 26, 2012 at 02:37:26PM +0200, Tomi Valkeinen wrote:
On 2012-11-26 11:07, Steffen Trumtrar wrote:
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+ u32 pixelclock; /* pixelclock in Hz */
I don't know if this is of any importance, but the linux clock framework
manages clock rates with unsigned long. Would it be better to use the
same type here?
Hm, I don't know. Anyone? u32 should be large enough for a pixelclock.
4GHz is a pretty large pixel clock. I have no idea how conceivable it is
that hardware will get to that speed. However, if it will ever be
larger, then you'll need to account for that in the DT binding so that
the pixel clock can be specified using 2 cells.
g.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 1/6] video: add display_timing and videomode
On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar s.trumt...@pengutronix.de wrote: Add display_timing structure and the according helper functions. This allows the description of a display via its supported timing parameters. Every timing parameter can be specified as a single value or a range min typ max. Also, add helper functions to convert from display timings to a generic videomode structure. This videomode can then be converted to the corresponding subsystem mode representation (e.g. fb_videomode). Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm making an incorrect assumption. It looks to me that the purpose of this entire series is to decode video timings from the device tree and (eventually) provide the data in the form 'struct videomode'. Correct? If so, then it looks over engineered. Creating new infrastructure to allocate, maintain, and free a new 'struct display_timings' doesn't make any sense when it is an intermediary data format that will never be used by drivers. Can the DT parsing code instead return a table of struct videomode? But, wait... struct videomode is also a new structure. So it looks like this series creates two new intermediary data structures; display_timings and videomode. And at least as far as I can see in this series struct fb_videomode is the only user. g. -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 1/6] video: add display_timing and videomode
On Thu, 15 Nov 2012 17:00:57 +0100, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Grant, On Thursday 15 November 2012 15:47:53 Grant Likely wrote: On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote: Add display_timing structure and the according helper functions. This allows the description of a display via its supported timing parameters. Every timing parameter can be specified as a single value or a range min typ max. Also, add helper functions to convert from display timings to a generic videomode structure. This videomode can then be converted to the corresponding subsystem mode representation (e.g. fb_videomode). Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm making an incorrect assumption. It looks to me that the purpose of this entire series is to decode video timings from the device tree and (eventually) provide the data in the form 'struct videomode'. Correct? If so, then it looks over engineered. Creating new infrastructure to allocate, maintain, and free a new 'struct display_timings' doesn't make any sense when it is an intermediary data format that will never be used by drivers. Can the DT parsing code instead return a table of struct videomode? But, wait... struct videomode is also a new structure. So it looks like this series creates two new intermediary data structures; display_timings and videomode. And at least as far as I can see in this series struct fb_videomode is the only user. struct videomode is supposed to slowly replace the various video mode structures we currently have in the kernel (struct drm_mode_modeinfo, struct fb_videomode and struct v4l2_bt_timings), at least where possible (userspace APIs can't be broken). This will make it possible to reuse code across the DRM, FB and V4L2 subsystems, such as the EDID parser or HDMI encoder drivers. This rationale might not be clearly explained in the commit message, but having a shared video mode structure is pretty important. Okay that make sense. What about struct display_timings? g. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 1/6] video: add display_timing and videomode
On Thu, 15 Nov 2012 17:00:57 +0100, Laurent Pinchart wrote: > Hi Grant, > > On Thursday 15 November 2012 15:47:53 Grant Likely wrote: > > On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote: > > > Add display_timing structure and the according helper functions. This > > > allows the description of a display via its supported timing parameters. > > > > > > Every timing parameter can be specified as a single value or a range > > > . > > > > > > Also, add helper functions to convert from display timings to a generic > > > videomode structure. This videomode can then be converted to the > > > corresponding subsystem mode representation (e.g. fb_videomode). > > > > > > Signed-off-by: Steffen Trumtrar > > > > Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm > > making an incorrect assumption. > > > > It looks to me that the purpose of this entire series is to decode video > > timings from the device tree and (eventually) provide the data in the > > form 'struct videomode'. Correct? > > > > If so, then it looks over engineered. Creating new infrastructure to > > allocate, maintain, and free a new 'struct display_timings' doesn't make > > any sense when it is an intermediary data format that will never be used > > by drivers. > > > > Can the DT parsing code instead return a table of struct videomode? > > > > But, wait... struct videomode is also a new structure. So it looks like > > this series creates two new intermediary data structures; > > display_timings and videomode. And at least as far as I can see in this > > series struct fb_videomode is the only user. > > struct videomode is supposed to slowly replace the various video mode > structures we currently have in the kernel (struct drm_mode_modeinfo, struct > fb_videomode and struct v4l2_bt_timings), at least where possible (userspace > APIs can't be broken). This will make it possible to reuse code across the > DRM, FB and V4L2 subsystems, such as the EDID parser or HDMI encoder drivers. > This rationale might not be clearly explained in the commit message, but > having a shared video mode structure is pretty important. Okay that make sense. What about struct display_timings? g.
[PATCH v10 1/6] video: add display_timing and videomode
On Thu, 15 Nov 2012 10:23:52 +0100, Steffen Trumtrar wrote: > Add display_timing structure and the according helper functions. This allows > the description of a display via its supported timing parameters. > > Every timing parameter can be specified as a single value or a range > . > > Also, add helper functions to convert from display timings to a generic > videomode > structure. This videomode can then be converted to the corresponding subsystem > mode representation (e.g. fb_videomode). > > Signed-off-by: Steffen Trumtrar Hmmm... here's my thoughts as an outside reviewer. Correct me if I'm making an incorrect assumption. It looks to me that the purpose of this entire series is to decode video timings from the device tree and (eventually) provide the data in the form 'struct videomode'. Correct? If so, then it looks over engineered. Creating new infrastructure to allocate, maintain, and free a new 'struct display_timings' doesn't make any sense when it is an intermediary data format that will never be used by drivers. Can the DT parsing code instead return a table of struct videomode? But, wait... struct videomode is also a new structure. So it looks like this series creates two new intermediary data structures; display_timings and videomode. And at least as far as I can see in this series struct fb_videomode is the only user. g. -- Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd.
Re: [PATCH] i915: Quirk out disconnected backlight
On Mon, Sep 17, 2012 at 9:03 AM, Jani Nikula jani.nik...@linux.intel.com wrote: Hi Grant, please try v3.6-rc6 that does exactly that with: commit 28dcc2d60cb570d9f549c329b2f51400553412a1 Author: Jani Nikula jani.nik...@intel.com Date: Mon Sep 3 16:25:12 2012 +0300 drm/i915: do not expose a dysfunctional backlight interface to userspace Does that fix it for you? Yes, thanks. g. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] i915: Quirk out disconnected backlight
On Mon, Sep 17, 2012 at 9:03 AM, Jani Nikula wrote: > Hi Grant, please try v3.6-rc6 that does exactly that with: > > commit 28dcc2d60cb570d9f549c329b2f51400553412a1 > Author: Jani Nikula > Date: Mon Sep 3 16:25:12 2012 +0300 > > drm/i915: do not expose a dysfunctional backlight interface to userspace > > Does that fix it for you? Yes, thanks. g.
Re: [PATCH] i915: Quirk out disconnected backlight
On Fri, Sep 14, 2012 at 2:01 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Fri, 14 Sep 2012 13:57:06 +0100, Grant Likely grant.lik...@secretlab.ca wrote: Some platforms (for instance MacbookPros) have custom backlight drivers and don't use the integrated i915 backlight control. This patch adds a quirk to disable registering the intel backlight when unused on a platform. Tested on MacbookPro8,3. Without this patch both the intel_backlight and gmux_backlight devices get registered and userspace doesn't know which it should use. Userspace is informed throught the backlight/type property. Perhaps, but userspace (Ubuntu) isn't doing anything with it, and it still remains that it makes no sense whatsoever to register a backlight device that doesn't exist. g. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] i915: Quirk out disconnected backlight
Some platforms (for instance MacbookPros) have custom backlight drivers
and don't use the integrated i915 backlight control. This patch adds a
quirk to disable registering the intel backlight when unused on a
platform.
Tested on MacbookPro8,3. Without this patch both the intel_backlight and
gmux_backlight devices get registered and userspace doesn't know which
it should use.
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: David Airlie airl...@linux.ie
Cc: Matthew Garrett m...@redhat.com
Cc: David Woodhouse dw...@infradead.org
---
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/intel_display.c | 10 ++
drivers/gpu/drm/i915/intel_panel.c |3 +++
3 files changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 627fe35..48860a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -349,6 +349,7 @@ enum intel_pch {
#define QUIRK_PIPEA_FORCE (10)
#define QUIRK_LVDS_SSC_DISABLE (11)
#define QUIRK_INVERT_BRIGHTNESS (12)
+#define QUIRK_NO_BACKLIGHT (22)
struct intel_fbdev;
struct intel_fbc_work;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2dfa6cf..c8153cd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7088,6 +7088,13 @@ static void quirk_invert_brightness(struct drm_device
*dev)
DRM_INFO(applying inverted panel brightness quirk\n);
}
+static void quirk_no_backlight(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ dev_priv-quirks |= QUIRK_NO_BACKLIGHT;
+ DRM_INFO(applying no backlight quirk\n);
+}
+
struct intel_quirk {
int device;
int subsystem_vendor;
@@ -7123,6 +7130,9 @@ static struct intel_quirk intel_quirks[] = {
/* Acer Aspire 5734Z must invert backlight brightness */
{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
+
+ /* Apple MacbookPro8,3 doesn't have a backlight */
+ { 0x0126, 0x106b, 0x00de, quirk_no_backlight },
};
static void intel_init_quirks(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 3df4f5f..f116e2a 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -413,6 +413,9 @@ int intel_panel_setup_backlight(struct drm_device *dev)
struct backlight_properties props;
struct drm_connector *connector;
+ if (dev_priv-quirks QUIRK_NO_BACKLIGHT)
+ return 0;
+
intel_panel_init_backlight(dev);
if (dev_priv-int_lvds_connector)
--
1.7.9.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] i915: Quirk out disconnected backlight
On Fri, Sep 14, 2012 at 2:12 PM, David Woodhouse dw...@infradead.org wrote:
On Fri, 2012-09-14 at 14:09 +0100, Grant Likely wrote:
On Fri, Sep 14, 2012 at 2:01 PM, Chris Wilson ch...@chris-wilson.co.uk
wrote:
On Fri, 14 Sep 2012 13:57:06 +0100, Grant Likely
grant.lik...@secretlab.ca wrote:
Some platforms (for instance MacbookPros) have custom backlight drivers
and don't use the integrated i915 backlight control. This patch adds a
quirk to disable registering the intel backlight when unused on a
platform.
Tested on MacbookPro8,3. Without this patch both the intel_backlight and
gmux_backlight devices get registered and userspace doesn't know which
it should use.
Userspace is informed throught the backlight/type property.
Perhaps, but userspace (Ubuntu) isn't doing anything with it, and it
still remains that it makes no sense whatsoever to register a
backlight device that doesn't exist.
Indeed. Userspace (well, gnome-settings-daemon) will use the backlight
provided by X, in preference to anything it finds
in /sys/class/backlight. So if the Intel one is present (and thus
exposed via X) then userspace will never bother with comparing types and
choosing the sanest backlight to use.
See https://bugzilla.redhat.com/show_bug.cgi?id=752595
In that bug you mention that the intel backlight sets a bogus max of
'1' when a backlight isn't present. I saw that too here. Here's the
offending code:
u32 intel_panel_get_max_backlight(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
u32 max;
max = i915_read_blc_pwm_ctl(dev_priv);
if (max == 0) {
/* XXX add code here to query mode clock or hardware clock
* and program max PWM appropriately.
*/
pr_warn_once(fixme: max PWM is zero\n);
return 1;
}
I used a quirk in my patch, but I could instead change the driver to
bail here instead of trying to limp along.
g.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] i915: Don't register backlight when max PWM value is unknown
When a backlight isn't connected to the i915 it doesn't make any sense
to register the backlight device, but the driver currently tries to limp
along using a max brightness value of 1. Instead, this patch makes it so
that if the maximum PWM value cannot be determined, then the backlight
will not be registered.
Tested on MacbookPro8,3.
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: David Airlie airl...@linux.ie
Cc: Matthew Garrett m...@redhat.com
Cc: David Woodhouse dw...@infradead.org
---
drivers/gpu/drm/i915/intel_panel.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 3df4f5f..f410c6e 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -168,13 +168,8 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev)
u32 max;
max = i915_read_blc_pwm_ctl(dev_priv);
- if (max == 0) {
- /* XXX add code here to query mode clock or hardware clock
-* and program max PWM appropriately.
-*/
- pr_warn_once(fixme: max PWM is zero\n);
- return 1;
- }
+ if (max == 0)
+ return 0; /* Cannot read max PWM. Assume no backlight */
if (HAS_PCH_SPLIT(dev)) {
max = 16;
@@ -413,6 +408,12 @@ int intel_panel_setup_backlight(struct drm_device *dev)
struct backlight_properties props;
struct drm_connector *connector;
+ /* Is there a backlight present? max will be zero if not */
+ if (intel_panel_get_max_backlight(dev) == 0) {
+ DRM_INFO(i915 doesn't seem to be connected to backlight\n);
+ return 0;
+ }
+
intel_panel_init_backlight(dev);
if (dev_priv-int_lvds_connector)
--
1.7.9.5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] i915: Don't register backlight when max PWM value is unknown
When a backlight isn't connected to the i915 it doesn't make any sense
to register the backlight device, but the driver currently tries to limp
along using a max brightness value of 1. Instead, this patch makes it so
that if the maximum PWM value cannot be determined, then the backlight
will not be registered.
Tested on MacbookPro8,3.
Signed-off-by: Grant Likely
Cc: Daniel Vetter
Cc: David Airlie
Cc: Matthew Garrett
Cc: David Woodhouse
---
drivers/gpu/drm/i915/intel_panel.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 3df4f5f..f410c6e 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -168,13 +168,8 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev)
u32 max;
max = i915_read_blc_pwm_ctl(dev_priv);
- if (max == 0) {
- /* XXX add code here to query mode clock or hardware clock
-* and program max PWM appropriately.
-*/
- pr_warn_once("fixme: max PWM is zero\n");
- return 1;
- }
+ if (max == 0)
+ return 0; /* Cannot read max PWM. Assume no backlight */
if (HAS_PCH_SPLIT(dev)) {
max >>= 16;
@@ -413,6 +408,12 @@ int intel_panel_setup_backlight(struct drm_device *dev)
struct backlight_properties props;
struct drm_connector *connector;
+ /* Is there a backlight present? max will be zero if not */
+ if (intel_panel_get_max_backlight(dev) == 0) {
+ DRM_INFO("i915 doesn't seem to be connected to backlight\n");
+ return 0;
+ }
+
intel_panel_init_backlight(dev);
if (dev_priv->int_lvds_connector)
--
1.7.9.5
[PATCH] i915: Quirk out disconnected backlight
On Fri, Sep 14, 2012 at 2:12 PM, David Woodhouse wrote:
> On Fri, 2012-09-14 at 14:09 +0100, Grant Likely wrote:
>> On Fri, Sep 14, 2012 at 2:01 PM, Chris Wilson
>> wrote:
>> > On Fri, 14 Sep 2012 13:57:06 +0100, Grant Likely > > secretlab.ca> wrote:
>> >> Some platforms (for instance MacbookPros) have custom backlight drivers
>> >> and don't use the integrated i915 backlight control. This patch adds a
>> >> quirk to disable registering the intel backlight when unused on a
>> >> platform.
>> >>
>> >> Tested on MacbookPro8,3. Without this patch both the intel_backlight and
>> >> gmux_backlight devices get registered and userspace doesn't know which
>> >> it should use.
>> >
>> > Userspace is informed throught the backlight/type property.
>>
>> Perhaps, but userspace (Ubuntu) isn't doing anything with it, and it
>> still remains that it makes no sense whatsoever to register a
>> backlight device that doesn't exist.
>
> Indeed. Userspace (well, gnome-settings-daemon) will use the backlight
> provided by X, in preference to anything it finds
> in /sys/class/backlight. So if the Intel one is present (and thus
> exposed via X) then userspace will never bother with comparing types and
> choosing the sanest backlight to use.
>
> See https://bugzilla.redhat.com/show_bug.cgi?id=752595
In that bug you mention that the intel backlight sets a bogus max of
'1' when a backlight isn't present. I saw that too here. Here's the
offending code:
u32 intel_panel_get_max_backlight(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
u32 max;
max = i915_read_blc_pwm_ctl(dev_priv);
if (max == 0) {
/* XXX add code here to query mode clock or hardware clock
* and program max PWM appropriately.
*/
pr_warn_once("fixme: max PWM is zero\n");
return 1;
}
I used a quirk in my patch, but I could instead change the driver to
bail here instead of trying to limp along.
g.
[PATCH] i915: Quirk out disconnected backlight
On Fri, Sep 14, 2012 at 2:01 PM, Chris Wilson wrote: > On Fri, 14 Sep 2012 13:57:06 +0100, Grant Likely secretlab.ca> wrote: >> Some platforms (for instance MacbookPros) have custom backlight drivers >> and don't use the integrated i915 backlight control. This patch adds a >> quirk to disable registering the intel backlight when unused on a >> platform. >> >> Tested on MacbookPro8,3. Without this patch both the intel_backlight and >> gmux_backlight devices get registered and userspace doesn't know which >> it should use. > > Userspace is informed throught the backlight/type property. Perhaps, but userspace (Ubuntu) isn't doing anything with it, and it still remains that it makes no sense whatsoever to register a backlight device that doesn't exist. g. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
[PATCH] i915: Quirk out disconnected backlight
Some platforms (for instance MacbookPros) have custom backlight drivers
and don't use the integrated i915 backlight control. This patch adds a
quirk to disable registering the intel backlight when unused on a
platform.
Tested on MacbookPro8,3. Without this patch both the intel_backlight and
gmux_backlight devices get registered and userspace doesn't know which
it should use.
Signed-off-by: Grant Likely
Cc: Daniel Vetter
Cc: David Airlie
Cc: Matthew Garrett
Cc: David Woodhouse
---
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/intel_display.c | 10 ++
drivers/gpu/drm/i915/intel_panel.c |3 +++
3 files changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 627fe35..48860a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -349,6 +349,7 @@ enum intel_pch {
#define QUIRK_PIPEA_FORCE (1<<0)
#define QUIRK_LVDS_SSC_DISABLE (1<<1)
#define QUIRK_INVERT_BRIGHTNESS (1<<2)
+#define QUIRK_NO_BACKLIGHT (2<<2)
struct intel_fbdev;
struct intel_fbc_work;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2dfa6cf..c8153cd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7088,6 +7088,13 @@ static void quirk_invert_brightness(struct drm_device
*dev)
DRM_INFO("applying inverted panel brightness quirk\n");
}
+static void quirk_no_backlight(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ dev_priv->quirks |= QUIRK_NO_BACKLIGHT;
+ DRM_INFO("applying no backlight quirk\n");
+}
+
struct intel_quirk {
int device;
int subsystem_vendor;
@@ -7123,6 +7130,9 @@ static struct intel_quirk intel_quirks[] = {
/* Acer Aspire 5734Z must invert backlight brightness */
{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
+
+ /* Apple MacbookPro8,3 doesn't have a backlight */
+ { 0x0126, 0x106b, 0x00de, quirk_no_backlight },
};
static void intel_init_quirks(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 3df4f5f..f116e2a 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -413,6 +413,9 @@ int intel_panel_setup_backlight(struct drm_device *dev)
struct backlight_properties props;
struct drm_connector *connector;
+ if (dev_priv->quirks & QUIRK_NO_BACKLIGHT)
+ return 0;
+
intel_panel_init_backlight(dev);
if (dev_priv->int_lvds_connector)
--
1.7.9.5
