Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Sat, Sep 20, 2014 at 8:57 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: [adding Kukjin as cc] Hello Ajay, On Sat, Sep 20, 2014 at 1:22 PM, Ajay kumar ajayn...@gmail.com wrote: Generally speaking, I sense that we have different views of how display devices and drivers are structured. You say If some XYZ platform wishes to pick the DT node via a different method, they are always welcome to do it.. This sounds to me that you see the connections between display devices as something handled by a platform specific driver. I, on the other hand, see connections between display devices as common properties. Well, I am okay with using video ports to describe the relationship between the encoder, bridge and the panel. But, its just that I need to make use of 2 functions when phandle does it using just one function ;) -panel_node = of_parse_phandle(dev-of_node, panel, 0) + endpoint = of_graph_get_next_endpoint(dev-of_node, NULL); + if (!endpoint) + return -EPROBE_DEFER; + + panel_node = of_graph_get_remote_port_parent(endpoint); + if (!panel_node) + return -EPROBE_DEFER; If nobody else has objections over using of_graph functions instead of phandles, I can respin this patchset by making use of video ports. I certainly have no objections about re-using the video ports/endpoints DT bindings for the bridges but just wanted to point out that Kukjin has already merged on his tree the DTS changes for display on Snow and Peach Pit using the current binding and also sent the pull request [0] to ARM SoC maintainers since he probably was expecting this series to make ti for 3.18. So that should be handled somehow. Kukjin, Can you do something about this? Or, I shall make video-port changes on top of whatever gets merged? Ajay Tomi, I see that Documentation/devicetree/bindings/video/ti,omap-dss.txt mentions that the Video Ports binding documentation is in Documentation/devicetree/bindings/video/video-ports.txt but I don't see that this file exists in the kernel [1]. I see though that is was included on your series adding DSS DT support [2]. Do you know what happened with this file? Best regards, Javier [0]: https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg36681.html [1]: https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/video [2]: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-January/227088.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts
Hi Tomasz, On Sat, Sep 13, 2014 at 4:57 PM, Tomasz Figa tomasz.f...@gmail.com wrote: Hi Abhilash, Please see my comments inline. On 13.09.2014 10:50, Abhilash Kesavan wrote: Exynos7 uses different offsets for wakeup interrupt configuration registers. So a new irq_chip instance for Exynos7 wakeup interrupts is added. The irq_chip selection is now based on the wakeup interrupt controller compatible string. [snip] @@ -328,9 +322,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on) /* * irq_chip for wakeup interrupts */ -static struct exynos_irq_chip exynos_wkup_irq_chip = { +static struct exynos_irq_chip exynos_wkup_irq_chip; + Why do you still need this, if you have both variants below? After adding __initdata to the two variants, I will require to have a copy of one of them. +static struct exynos_irq_chip exynos4210_wkup_irq_chip = { .chip = { - .name = exynos_wkup_irq_chip, + .name = exynos4210_wkup_irq_chip, .irq_unmask = exynos_irq_unmask, .irq_mask = exynos_irq_mask, .irq_ack = exynos_irq_ack, @@ -342,6 +338,29 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = { .eint_pend = EXYNOS_WKUP_EPEND_OFFSET, }; +static struct exynos_irq_chip exynos7_wkup_irq_chip = { + .chip = { + .name = exynos7_wkup_irq_chip, + .irq_unmask = exynos_irq_unmask, + .irq_mask = exynos_irq_mask, + .irq_ack = exynos_irq_ack, + .irq_set_type = exynos_irq_set_type, + .irq_set_wake = exynos_wkup_irq_set_wake, + }, + .eint_con = EXYNOS7_WKUP_ECON_OFFSET, + .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET, + .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET, +}; + +/* list of external wakeup controllers supported */ +static const struct of_device_id exynos_wkup_irq_ids[] = { + { .compatible = samsung,exynos4210-wakeup-eint, + .data = exynos4210_wkup_irq_chip }, + { .compatible = samsung,exynos7-wakeup-eint, + .data = exynos7_wkup_irq_chip }, + { } +}; + /* interrupt handler for wakeup interrupts 0..15 */ static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc) { @@ -434,7 +453,12 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) int idx, irq; for_each_child_of_node(dev-of_node, np) { - if (of_match_node(exynos_wkup_irq_ids, np)) { + const struct of_device_id *match; + + match = of_match_node(exynos_wkup_irq_ids, np); + if (match) { + memcpy(exynos_wkup_irq_chip, match-data, + sizeof(struct exynos_irq_chip)); Hmm, this doesn't look correct to me. You are modifying a static struct here. Why couldn't you simply use the exynos irq chip pointed by match-data in further registration code? That will not be available later once I use __initdata. wkup_np = np; break; } diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h index e060722..0db1e52 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -25,6 +25,9 @@ #define EXYNOS_WKUP_ECON_OFFSET 0xE00 #define EXYNOS_WKUP_EMASK_OFFSET 0xF00 #define EXYNOS_WKUP_EPEND_OFFSET 0xF40 +#define EXYNOS7_WKUP_ECON_OFFSET 0x700 +#define EXYNOS7_WKUP_EMASK_OFFSET0x900 +#define EXYNOS7_WKUP_EPEND_OFFSET0xA00 Interestingly enough, the offsets look just like the normal GPIO interrupt controller of previous Exynos SoCs. Are you sure those are correct? Also if somehow the controller now resembles the normal one, doesn't it have the SVC register making it possible to reuse the non wake-up code instead? The wakeup interrupt register offsets are the same as the GPIO interrupt offsets in earlier Exynos SoCs. There is no SVC register for the wakeup interrupt block. Regards, Abhilash Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: dts: Add rtc_src clk for s3c-rtc on exynos Peach boards
Dear Javier, On 09/17/2014 08:50 PM, Javier Martinez Canillas wrote: commit 546b117fdf17 (rtc: s3c: add support for RTC of Exynos3250 SoC) added an rtc_src DT property for the Samsung's S3C Real Time Clock controller that specifies the 32.768 kHz clock that uses the RTC as its source clock. In the case of the Peach Pit and Pi machines, the Maxim 77802 32kHz AP clock is used as the source clock. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 5 - arch/arm/boot/dts/exynos5800-peach-pi.dts | 5 - 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts index 9a23382..bfd189e 100644 --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts @@ -12,6 +12,7 @@ #include dt-bindings/input/input.h #include dt-bindings/gpio/gpio.h #include dt-bindings/interrupt-controller/irq.h +#include dt-bindings/clock/maxim,max77802.h #include exynos5420.dtsi / { @@ -151,7 +152,7 @@ status = okay; clock-frequency = 40; - max77802-pmic@9 { + max77802: max77802-pmic@9 { compatible = maxim,max77802; interrupt-parent = gpx3; interrupts = 1 IRQ_TYPE_NONE; @@ -727,6 +728,8 @@ rtc { status = okay; + clocks = clock CLK_RTC, max77802 MAX77802_CLK_32K_AP; + clock-names = rtc, rtc_src; }; spi_2 { diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts index 1d31c81..84ec1ce 100644 --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts @@ -12,6 +12,7 @@ #include dt-bindings/input/input.h #include dt-bindings/gpio/gpio.h #include dt-bindings/interrupt-controller/irq.h +#include dt-bindings/clock/maxim,max77802.h #include exynos5800.dtsi / { @@ -150,7 +151,7 @@ status = okay; clock-frequency = 40; - max77802-pmic@9 { + max77802: max77802-pmic@9 { compatible = maxim,max77802; interrupt-parent = gpx3; interrupts = 1 IRQ_TYPE_NONE; @@ -715,6 +716,8 @@ rtc { status = okay; + clocks = clock CLK_RTC, max77802 MAX77802_CLK_32K_AP; + clock-names = rtc, rtc_src; }; spi_2 { I'm so sorry for delay reply because I'm out of office for vacation (9/5 ~ 9/20). Looks goot to me for this patch. Reviewed-by: Chanwoo Choi cw00.c...@samsung.com Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: dts: Add rtc_src clk for s3c-rtc on exynos5250-snow
Dear Javier, On 09/17/2014 08:50 PM, Javier Martinez Canillas wrote: commit 546b117fdf17 (rtc: s3c: add support for RTC of Exynos3250 SoC) added an rtc_src DT property for the Samsung's S3C Real Time Clock controller that specifies the 32.768 kHz clock that uses the RTC as its source clock. In the case of the Exynos5250 based Snow board, the Maxim 77686 32kHz AP clock is used as the source clock. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5250-snow.dts | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts index e51fcef..56eec56 100644 --- a/arch/arm/boot/dts/exynos5250-snow.dts +++ b/arch/arm/boot/dts/exynos5250-snow.dts @@ -10,6 +10,7 @@ /dts-v1/; #include dt-bindings/gpio/gpio.h +#include dt-bindings/clock/maxim,max77686.h #include exynos5250.dtsi / { @@ -29,6 +30,8 @@ rtc@101E { status = okay; + clocks = clock CLK_RTC, max77686 MAX77686_CLK_AP; + clock-names = rtc, rtc_src; }; pinctrl@1140 { @@ -350,7 +353,7 @@ samsung,i2c-sda-delay = 100; samsung,i2c-max-bus-freq = 378000; - max77686@09 { + max77686: max77686@09 { compatible = maxim,max77686; interrupt-parent = gpx3; interrupts = 2 0; Reviewed-by: Chanwoo Choi cw00.c...@samsung.com Thanks, Chanwoo Choi -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6 6/8] drm/bridge: Modify drm_bridge core to support driver model
On Wed, Sep 17, 2014 at 12:27:13PM +0300, Laurent Pinchart wrote: Hi Ajay, On Wednesday 17 September 2014 14:37:30 Ajay kumar wrote: On Mon, Sep 15, 2014 at 11:07 PM, Laurent Pinchart wrote: Hi Ajay, Thank you for the patch. I think we're moving in the right direction, but we're not there yet. On Saturday 26 July 2014 00:52:08 Ajay Kumar wrote: This patch tries to seperate drm_bridge implementation into 2 parts, a drm part and a non_drm part. A set of helper functions are defined in this patch to make bridge driver probe independent of the drm flow. The bridge devices register themselves on a lookup table when they get probed by calling drm_bridge_add_for_lookup. The parent encoder driver waits till the bridge is available in the lookup table(by calling of_drm_find_bridge) and then continues with its initialization. Before the introduction of the component framework I would have said this is the way to go. Now, I think bridges should register themselves as components, and the DRM master driver should use the component framework to get a reference to the bridges it needs. Well, I have modified the bridge framework exactly the way Thierry wanted it to be, I mean the same way the current panel framework is. And, I don't think there is a problem with that. What problem are you facing with current bridge implementation? What is the advantage of using the component framework to register bridges? There are several advantages. - The component framework has been designed with this exact problem in mind, piecing multiple components into a display device. No. Component framework was designed with multi-device drivers in mind. That is, drivers that need to combine two or more platform devices into a single logical device. Typically that includes display controllers and encoders (in various looks) for DRM. Panels and bridges are in my opinion different because they are outside of the DRM driver. They aren't part of the device complex that an SoC provides. They represent hardware that is external to the SoC and the DRM driver and can be shared across SoCs. Forcing panels and bridges to register as components will require all drivers to implement master/component support solely for accessing this external hardware. What you're suggesting is like saying that clocks or regulators should register as components so that their users can get them that way. In fact by that argument everything that's referenced by phandle would need to register as component (PHYs, LEDs, GPIOs, I2C controllers, ...). This patch set introduces yet another framework, without any compelling reason as far as I can see. Today DRM drivers already need to use three different frameworks (component, I2C slave encoder and panel), and we're adding a fourth one to make the mess even messier. Panel and bridge aren't really frameworks. Rather they are a simple registry to allow drivers to register panels and bridges and display drivers to look them up. This is really a headlong rush, we need to stop and fix the design mistakes. Can you point out specific design mistakes? I don't see any, but I'm obviously biased. - The component framework solves the probe ordering problem. Bridges can use deferred probing, but when a bridge requires a resources (such as a clock for instance) provided by the display controller, this will break. Panel and bridges can support deferred probing without the component framework just fine. Without this patchset, you cannot bring an X server based display on snow and peach_pit. Also, day by day the number of platforms using drm_bridge is increasing. That's exactly why I'd like to use the component framework now, as the conversion will become more complex as time goes by. No it won't. If we ever do decide that component framework is a better fit then the conversion may be more work but it would still be largely mechanical. Thierry pgpQjJajyVwfv.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Wed, Sep 17, 2014 at 02:52:42PM +0300, Tomi Valkeinen wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? + +Example: + lvds-bridge@48 { + compatible = parade,ps8622; + reg = 0x48; + sleep-gpios = gpc3 6 1 0 0; + reset-gpios = gpc3 1 1 0 0; + lane-count = 1; + }; I wish all new display component bindings would use the video ports/endpoints to describe the connections. It will be very difficult to improve the display driver model later if we're missing such critical pieces from the DT bindings. I disagree. Why would we want to burden all devices with a bloated binding and drivers with parsing a complex graph when it's not even known that it will be necessary? Evidently this device works fine using the current binding. Just because there are bindings to describe ports in a generic way doesn't mean it has to be applied everywhere. After all the concept of ports/endpoints applies to non-video devices too, yet we don't require the bindings for those devices to add ports or endpoints nodes. Also it won't be very difficult to extend the binding in a backwards compatible way if that becomes necessary. One thing that I'd like to see in this binding, though, is how to hook up the bridge to a panel. However I'm still catching up on mail after vacation, so perhaps this has already been discussed further down the thread. Thierry pgp3rIVO_9uoT.pgp Description: PGP signature
Re: [PATCH 2/4] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts
On 22.09.2014 08:17, Abhilash Kesavan wrote: Hi Tomasz, On Sat, Sep 13, 2014 at 4:57 PM, Tomasz Figa tomasz.f...@gmail.com wrote: Hi Abhilash, Please see my comments inline. On 13.09.2014 10:50, Abhilash Kesavan wrote: Exynos7 uses different offsets for wakeup interrupt configuration registers. So a new irq_chip instance for Exynos7 wakeup interrupts is added. The irq_chip selection is now based on the wakeup interrupt controller compatible string. [snip] @@ -328,9 +322,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on) /* * irq_chip for wakeup interrupts */ -static struct exynos_irq_chip exynos_wkup_irq_chip = { +static struct exynos_irq_chip exynos_wkup_irq_chip; + Why do you still need this, if you have both variants below? After adding __initdata to the two variants, I will require to have a copy of one of them. +static struct exynos_irq_chip exynos4210_wkup_irq_chip = { .chip = { - .name = exynos_wkup_irq_chip, + .name = exynos4210_wkup_irq_chip, .irq_unmask = exynos_irq_unmask, .irq_mask = exynos_irq_mask, .irq_ack = exynos_irq_ack, @@ -342,6 +338,29 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = { .eint_pend = EXYNOS_WKUP_EPEND_OFFSET, }; +static struct exynos_irq_chip exynos7_wkup_irq_chip = { + .chip = { + .name = exynos7_wkup_irq_chip, + .irq_unmask = exynos_irq_unmask, + .irq_mask = exynos_irq_mask, + .irq_ack = exynos_irq_ack, + .irq_set_type = exynos_irq_set_type, + .irq_set_wake = exynos_wkup_irq_set_wake, + }, + .eint_con = EXYNOS7_WKUP_ECON_OFFSET, + .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET, + .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET, +}; + +/* list of external wakeup controllers supported */ +static const struct of_device_id exynos_wkup_irq_ids[] = { + { .compatible = samsung,exynos4210-wakeup-eint, + .data = exynos4210_wkup_irq_chip }, + { .compatible = samsung,exynos7-wakeup-eint, + .data = exynos7_wkup_irq_chip }, + { } +}; + /* interrupt handler for wakeup interrupts 0..15 */ static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc) { @@ -434,7 +453,12 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) int idx, irq; for_each_child_of_node(dev-of_node, np) { - if (of_match_node(exynos_wkup_irq_ids, np)) { + const struct of_device_id *match; + + match = of_match_node(exynos_wkup_irq_ids, np); + if (match) { + memcpy(exynos_wkup_irq_chip, match-data, + sizeof(struct exynos_irq_chip)); Hmm, this doesn't look correct to me. You are modifying a static struct here. Why couldn't you simply use the exynos irq chip pointed by match-data in further registration code? That will not be available later once I use __initdata. Then either __initdata shouldn't be necessary or kmemdup() should be used to allocate a copy. wkup_np = np; break; } diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h index e060722..0db1e52 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -25,6 +25,9 @@ #define EXYNOS_WKUP_ECON_OFFSET 0xE00 #define EXYNOS_WKUP_EMASK_OFFSET 0xF00 #define EXYNOS_WKUP_EPEND_OFFSET 0xF40 +#define EXYNOS7_WKUP_ECON_OFFSET 0x700 +#define EXYNOS7_WKUP_EMASK_OFFSET0x900 +#define EXYNOS7_WKUP_EPEND_OFFSET0xA00 Interestingly enough, the offsets look just like the normal GPIO interrupt controller of previous Exynos SoCs. Are you sure those are correct? Also if somehow the controller now resembles the normal one, doesn't it have the SVC register making it possible to reuse the non wake-up code instead? The wakeup interrupt register offsets are the same as the GPIO interrupt offsets in earlier Exynos SoCs. There is no SVC register for the wakeup interrupt block. OK, your code is fine then. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Wed, Sep 17, 2014 at 07:22:05PM +0300, Tomi Valkeinen wrote: On 17/09/14 17:29, Ajay kumar wrote: Hi Tomi, Thanks for your comments. On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? use-external-pwm or disable-bridge-pwm would be better. Well, the properties are about the bridge. use-external-pwm means that the bridge uses an external PWM, which, if I understood correctly, is not what the property is about. disable-bridge-pwm is ok, but the bridge there is extra. The properties are about the bridge, so it's implicit. + +Example: + lvds-bridge@48 { + compatible = parade,ps8622; + reg = 0x48; + sleep-gpios = gpc3 6 1 0 0; + reset-gpios = gpc3 1 1 0 0; + lane-count = 1; + }; I wish all new display component bindings would use the video ports/endpoints to describe the connections. It will be very difficult to improve the display driver model later if we're missing such critical pieces from the DT bindings. Why do we need a complex graph when it can be handled using a simple phandle? Maybe in your case you can handle it with simple phandle. Can you guarantee that it's enough for everyone, on all platforms? Nobody can guarantee that. An interesting effect that DT ABI stability has had is that people now try to come up with overly generic concepts to describe devices in an attempt to make them more future-proof. I don't think we're doing ourselves a favour with that approach. The point of the ports/endpoint graph is to also support more complicated scenarios. But the scenario will always remain the same for this bridge. There will always be an input signal and a translation to some output signal along with some parameters to control how that translation happens. More complicated scenarios will likely need to be represented differently at a higher level than the bridge. If you now create ps8622 bindings that do not support those graphs, the no one else using ps8622 can use ports/endpoint graphs either. That's not true. The binding can easily be extended in a backwards- compatible way later on (should it really prove necessary). Thierry pgpz6xkbzUFzC.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Thu, Sep 18, 2014 at 11:20:40AM +0530, Ajay kumar wrote: Hi Tomi, On Wed, Sep 17, 2014 at 9:52 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 17/09/14 17:29, Ajay kumar wrote: Hi Tomi, Thanks for your comments. On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? use-external-pwm or disable-bridge-pwm would be better. Well, the properties are about the bridge. use-external-pwm means that the bridge uses an external PWM, which, if I understood correctly, is not what the property is about. disable-bridge-pwm is ok, but the bridge there is extra. The properties are about the bridge, so it's implicit. Ok. I will use disable-pwm. Why is this even necessary? According to the datasheet this device has circuitry for backlight control. If so, I'd expect it to expose either a backlight device or a PWM device. That way unless somebody is using the backlight/PWM exposed by the bridge the bridge can simply disable PWM. Thierry pgpMDBbDUCJ4e.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Fri, Sep 19, 2014 at 05:28:37PM +0300, Tomi Valkeinen wrote: On 19/09/14 16:59, Ajay kumar wrote: I am not really able to understand, what's stopping us from using this bridge on a board with complex display connections. To use ps8622 driver, one needs to attach it to the DRM framework. For this, the DRM driver Remember that when we talk about DT bindings, there's no such thing as DRM. We talk about hardware. The same bindings need to work on any operating system. would need the DT node for ps8622 bridge. For which I use a phandle. A complex one could be for example a case where you have two different panels connected to ps8622, and you can switch between the two panels with, say, a gpio. How do you present that with a simple phandle? How do you represent that with a graph? Whether you describe it using a graph or a simple phandle you'll need additional nodes somewhere in between. Perhaps something like this: mux: ... { compatible = gpio-mux-bridge; gpio = gpio 42 GPIO_ACTIVE_HIGH; panel@0 { panel = panel0; }; panel@1 { panel = panel1; }; }; ps8622: ... { bridge = mux; }; panel0: ... { ... }; panel1: ... { ... }; If some XYZ platform wishes to pick the DT node via a different method, they are always welcome to do it. Just because I am not specifying a video port/endpoint in the DT binding example, would it mean that platform cannot make use of ports in future? If that is the case, I can add something All the platforms share the same bindings for ps8622. If you now specify that ps8622 bindings use a simple phandle, then anyone who uses ps8622 should support that. Of course the bindings can be extended in the future. In that case the drivers need to support both the old and the new bindings, which is always a hassle. Generally speaking, I sense that we have different views of how display devices and drivers are structured. You say If some XYZ platform wishes to pick the DT node via a different method, they are always welcome to do it.. This sounds to me that you see the connections between display devices as something handled by a platform specific driver. I, on the other hand, see connections between display devices as common properties. Say, we could have a display board, with a panel and an encoder and maybe some other components, which takes parallel RGB as input. The same display board could as well be connected to an OMAP board or to an Exynos board. I think the exact same display-board.dtsi file, which describes the devices and connections in the display board, should be usable on both OMAP and Exynos platforms. This means we need to have a common way to describe video devices, just as we have for other things. A common way to describe devices in DT isn't going to give you that. The device tree is completely missing any information about how to access an extension board like that. The operating system defines the API by which the board can be accessed. I imagine that this would work by making the first component of the board a bridge of some sort and then every driver that supports that type of bridge (ideally just a generic drm_bridge) would also work with that display board. Whether this is described using a single phandle to the bridge or a more complicated graph is irrelevant. What matters is that you get a phandle to the bridge. The job of the operating system is to give drivers a way to resolve that phandle to some object and provide an API to access that object. Thierry pgpKcd8s1g3UB.pgp Description: PGP signature
Re: [PATCH v2 1/3] clk: samsung: exynos3250: Register DMC clk provider
On 02.09.2014 15:21, Krzysztof Kozlowski wrote: Add clock provider for clocks in DMC domain including EPLL and BPLL. The DMC clocks are necessary for Exynos3 devfreq driver. The DMC clock domain uses different address space (0x105C) than standard clock domain (0x1003 - 0x1005). The difference is huge enough to add new DT node for the clock provider, rather than extending existing address space. Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com --- Changes since v1: = 1. Fix overwritteing main clock provider reg_base with DMC clock domain reg_basr. This leads to OOPS in suspend. Applied the whole series for next. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
Hi Thierry, On Mon, Sep 22, 2014 at 1:40 PM, Thierry Reding thierry.red...@gmail.com wrote: On Thu, Sep 18, 2014 at 11:20:40AM +0530, Ajay kumar wrote: Hi Tomi, On Wed, Sep 17, 2014 at 9:52 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 17/09/14 17:29, Ajay kumar wrote: Hi Tomi, Thanks for your comments. On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? use-external-pwm or disable-bridge-pwm would be better. Well, the properties are about the bridge. use-external-pwm means that the bridge uses an external PWM, which, if I understood correctly, is not what the property is about. disable-bridge-pwm is ok, but the bridge there is extra. The properties are about the bridge, so it's implicit. Ok. I will use disable-pwm. Why is this even necessary? According to the datasheet this device has circuitry for backlight control. If so, I'd expect it to expose either a backlight device or a PWM device. That way unless somebody is using the backlight/PWM exposed by the bridge the bridge can simply disable PWM. The driver does expose a backlight device. And, the decision(whether to expose a backlight device or not) is made based on the DT flag use-external-pwm. This was discussed before, and you suggested to use the boolean property, refer to this link: http://lists.freedesktop.org/archives/dri-devel/2014-July/065048.html Ajay -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clk: samsung: exynos3250: fix width field of mout_mmc0/1
On 09.09.2014 14:14, Krzysztof Kozlowski wrote: On 05.09.2014 13:54, Pankaj Dubey wrote: As per Exynos3250 user manual mmc0/1 mux selection has 4 bit wide. Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com --- drivers/clk/samsung/clk-exynos3250.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Krzysztof Kozlowski k.kozlow...@samsung.com Applied for next. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clk: samsung: exynos3250: fix width and shift of div_spi0_isp clock
On 09.09.2014 14:14, Krzysztof Kozlowski wrote: On 09.09.2014 13:54, Pankaj Dubey wrote: Update shift and width field of div_spi0_isp clock as per Exynos3250 user manual. Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com --- drivers/clk/samsung/clk-exynos3250.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Krzysztof Kozlowski k.kozlow...@samsung.com Applied for next. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clk: samsung: exynos3250: fix mout_cam_blk parent list
On 09.09.2014 14:24, Krzysztof Kozłowski wrote: On 06.09.2014 15:03, Pankaj Dubey wrote: As per user manual of Exynos3250 SRC_CAM can select div_cam_blk_320 if it's value is 0xC, so placing div_cam_blk_320 at proper index in parent list of mout_cam_blk. Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com --- drivers/clk/samsung/clk-exynos3250.c |1 + 1 file changed, 1 insertion(+) Reviewed-by: Krzysztof Kozlowski k.kozlow...@samsung.com Applied for next. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clk: samsung: exynos5260: fix typo in clock name
On 22.09.2014 06:55, Pankaj Dubey wrote: Hi Tomasz, Will you please take this patch and following three Exynos3250 clock fixes in your tree. 1: clk: samsung: exynos3250: fix width field of mout_mmc0/1 https://lkml.org/lkml/2014/9/5/265 2: clk: samsung: exynos3250: fix width and shift of div_spi0_isp clock https://lkml.org/lkml/2014/9/9/364 3: clk: samsung: exynos3250: fix mout_cam_blk parent list https://lkml.org/lkml/2014/9/6/41 Applied all four patches for next. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 SoC
Hi Abhilash, On 22.09.2014 06:47, Abhilash Kesavan wrote: Changes since v4: - Fixed comments from Tomasz Figa: - Changed the namespace prefix from exynos to samsung - Defined bindings to take all input clocks - Sorted the Kconfig entries alphabetically in clock Makefile - Used consistent 1 tab line breaks across the clock file - Statically initialized the samsung_cmu_info struct - Enabled exynos7 in the arm64 defconfig as per Catalin Marinas' comment. - Added Kukjin Kim's ack along with Thomas Abraham's tested and reviewed tags. The clock patches look good to me, but since they are doing quite a lot of code moving I'd prefer to take them through clk tree. Based on the fact that there are no code dependencies between clock patches and remaining ones and Exynos7 is a new material for 3.18, I'm inclined to apply them to my tree if nobody minds. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm/exynos: remove explicit encoder/connector de-initialization
All KMS objects are destroyed by drm_mode_config_cleanup in proper order so component drivers should not care about it. Signed-off-by: Andrzej Hajda a.ha...@samsung.com --- Hi Inki, This is another spin-off of exynos_drm tests regarding your component support update. The patch is based as usual on the latest exynos-drm-next branch. Regards Andrzej --- drivers/gpu/drm/exynos/exynos_dp_core.c | 5 - drivers/gpu/drm/exynos/exynos_drm_dpi.c | 4 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 4 drivers/gpu/drm/exynos/exynos_drm_vidi.c | 3 --- drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- 5 files changed, 22 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index cd50ece..6adb1e5 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -1355,13 +1355,8 @@ static void exynos_dp_unbind(struct device *dev, struct device *master, void *data) { struct exynos_drm_display *display = dev_get_drvdata(dev); - struct exynos_dp_device *dp = display-ctx; - struct drm_encoder *encoder = dp-encoder; exynos_dp_dpms(display, DRM_MODE_DPMS_OFF); - - exynos_dp_connector_destroy(dp-connector); - encoder-funcs-destroy(encoder); } static const struct component_ops exynos_dp_ops = { diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c index 96c87db..3dc678e 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c @@ -338,14 +338,10 @@ err_del_component: int exynos_dpi_remove(struct device *dev) { - struct drm_encoder *encoder = exynos_dpi_display.encoder; struct exynos_dpi *ctx = exynos_dpi_display.ctx; exynos_dpi_dpms(exynos_dpi_display, DRM_MODE_DPMS_OFF); - exynos_dpi_connector_destroy(ctx-connector); - encoder-funcs-destroy(encoder); - if (ctx-panel) drm_panel_detach(ctx-panel); diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index 24741d8..acf7e9e 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1660,13 +1660,9 @@ static void exynos_dsi_unbind(struct device *dev, struct device *master, void *data) { struct exynos_dsi *dsi = exynos_dsi_display.ctx; - struct drm_encoder *encoder = dsi-encoder; exynos_dsi_dpms(exynos_dsi_display, DRM_MODE_DPMS_OFF); - exynos_dsi_connector_destroy(dsi-connector); - encoder-funcs-destroy(encoder); - mipi_dsi_host_unregister(dsi-dsi_host); } diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c index d565207..9395e85 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c @@ -639,9 +639,6 @@ static int vidi_remove(struct platform_device *pdev) return -EINVAL; } - encoder-funcs-destroy(encoder); - drm_connector_cleanup(ctx-connector); - return 0; } diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 7910fb3..563a19e 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -2312,12 +2312,6 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data) static void hdmi_unbind(struct device *dev, struct device *master, void *data) { - struct exynos_drm_display *display = get_hdmi_display(dev); - struct drm_encoder *encoder = display-encoder; - struct hdmi_context *hdata = display-ctx; - - hdmi_connector_destroy(hdata-connector); - encoder-funcs-destroy(encoder); } static const struct component_ops hdmi_component_ops = { -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] mfd: syscon: Decouple syscon interface from platform devices
On Mon, Sep 22, 2014 at 10:10:07AM +0530, Pankaj Dubey wrote: Currently a syscon entity can be only registered directly through a platform device that binds to a dedicated syscon driver. However in certain use cases it is desirable to make a device used with another driver a syscon interface provider. For example, certain SoCs (e.g. Exynos) contain system controller blocks which perform various functions such as power domain control, CPU power management, low power mode control, but in addition contain certain IP integration glue, such as various signal masks, coprocessor power control, etc. In such case, there is a need to have a dedicated driver for such system controller but also share registers with other drivers. The latter is where the syscon interface is helpful. In case of DT based platforms, this patch decouples syscon object from syscon platform driver, and allows to create syscon objects first time when it is required by calling of syscon_regmap_lookup_by APIs and keep a list of such syscon objects along with syscon provider device_nodes and regmap handles. For non-DT based platforms, this patch keeps syscon platform driver structure where is can be probed and such non-DT based drivers can use syscon_regmap_lookup_by_pdev API and get access to regmap handles. Once all users of syscon_regmap_lookup_by_pdev migrated to DT based, we can completly remove platform driver of syscon, and keep only helper functions to get regmap handles. Suggested-by: Arnd Bergmann a...@arndb.de Suggested-by: Tomasz Figa tomasz.f...@gmail.com Tested-by: Vivek Gautam gautam.vi...@samsung.com Tested-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com --- Patch v4 and related discussions can be found here [1]. Changes since v4: - Addressed Tomasz Figa's comments for v4. - Added error handing in of_syscon_register function. - Using devm_regmap_init_mmio instead of regmap_init_mmio. Changes since v3: - Addressed Arnd's comment for v2. - Updated of_syscon_register for adding dev pointer in regmap_init_mmio. - For early users created dummy platform device. Changes since v2: - Added back platform device support from syscon, with one change that syscon will not be probed for DT based platform. - Added back syscon_regmap_lookup_by_pdevname API so that non-DT base users of syscon will not be broken. - Removed unwanted change in syscon.h. - Modified Signed-off-by list, added Suggested-by of Tomasz Figa and Arnd Bergmann. - Added Tested-by of Vivek Gautam for testing on Exynos platform. Changes since v1: - Removed of_syscon_unregister function. - Modified of_syscon_register function and it will be used by syscon.c to create syscon objects whenever required. - Removed platform device support from syscon. - Removed syscon_regmap_lookup_by_pdevname API support. - As there are significant changes w.r.t patchset v1, I am taking over author for this patchset from Tomasz Figa. [1]: https://lkml.org/lkml/2014/9/19/267 drivers/mfd/syscon.c | 107 +++--- 1 file changed, 85 insertions(+), 22 deletions(-) diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c index ca15878..0b17f50 100644 --- a/drivers/mfd/syscon.c +++ b/drivers/mfd/syscon.c @@ -15,6 +15,7 @@ #include linux/err.h #include linux/io.h #include linux/module.h +#include linux/list.h #include linux/of.h #include linux/of_address.h #include linux/of_platform.h @@ -22,31 +23,105 @@ #include linux/platform_device.h #include linux/regmap.h #include linux/mfd/syscon.h +#include linux/slab.h static struct platform_driver syscon_driver; +static DEFINE_SPINLOCK(syscon_list_slock); +static LIST_HEAD(syscon_list); + struct syscon { + struct device_node *np; struct regmap *regmap; + struct list_head list; +}; + +static struct regmap_config syscon_regmap_config = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, }; -static int syscon_match_node(struct device *dev, void *data) +static struct syscon *of_syscon_register(struct device_node *np) { - struct device_node *dn = data; + struct platform_device *pdev = NULL; + struct syscon *syscon; + struct regmap *regmap; + void __iomem *base; + int ret; + + if (!of_device_is_compatible(np, syscon)) + return ERR_PTR(-EINVAL); + + syscon = kzalloc(sizeof(*syscon), GFP_KERNEL); + if (!syscon) + return ERR_PTR(-ENOMEM); + + base = of_iomap(np, 0); + if (!base) { + ret = -ENOMEM; + goto err_map; + } + + /* if device is already populated and available then use it */ + pdev = of_find_device_by_node(np); + if (!pdev) { + /* for early users create dummy syscon device and use it */ + pdev =
RE: [PATCH v5] mfd: syscon: Decouple syscon interface from platform devices
Hi, [...] +static struct regmap_config syscon_regmap_config = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, }; -static int syscon_match_node(struct device *dev, void *data) +static struct syscon *of_syscon_register(struct device_node *np) { - struct device_node *dn = data; + struct platform_device *pdev = NULL; struct platform_device *pdev; ? Thanks, BRs Xiubo -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 00/12] drm/exynos: few patches to enhance bridge chip support
On Wed, Sep 17, 2014 at 03:02:48PM +0530, Ajay kumar wrote: On Tue, Sep 16, 2014 at 6:14 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: [adding Laurent Pinchart to cc who had concerns with a previous version of this patch-set] Hello Ajay, On Wed, Aug 27, 2014 at 4:29 PM, Ajay Kumar ajaykumar...@samsung.com wrote: This series is based on master branch of Linus tree at: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git I have tested this after adding few DT changes for exynos5250-snow and exynos5420-peach-pit boards. The V4 series of this particular patchset was also tested by: Rahul Sharma rahul.sha...@samsung.com Javier Martinez Canillas jav...@dowhile0.org Changes since V2: -- Address comments from Jingoo Han for ps8622 driver -- Address comments from Daniel, Rob and Thierry regarding bridge chaining -- Address comments from Thierry regarding the names for new drm_panel functions Changes since V3: -- Remove hotplug based initialization of exynos_dp -- Make exynos_dp work directly with drm_panel, remove dependency on panel_binder -- Minor cleanups in panel_binder and panel_lvds driver Changes since V4: -- Use gpiod interface for panel-lvds and ps8622 drivers. -- Address comments from Javier. -- Fix compilation issues when PANEL_BINDER is selected as module. -- Split Documentation patches from driver patches. -- Rebase on top of the tree. Changes since V5: -- Modify bridge drivers to support driver model. -- Drop the concept of bridge chain(sincle there are no 2 real bridges) Hence drop bridge-panel_binder layer. -- Drop panel-lvds driver and accomodate the required changes in panel-simple driver. -- Use gpiod interface in ptn3460 driver. -- Address all comments by Thierry Reding for V5 series. -- Address comments from Sean Paul for exynos_dp_commit issue. Changes since V6: -- Panel patches were seperated and they are merged already. -- Fix few issues with ptn3460, before modifying the bridge core. -- Modify drm_bridge as per Thierry's comments for V6 series. -- Add drm_bridge changes minimally without breaking existing code. -- Add new features for ptn3460, step-by-step. -- Address comments from Thierry and Andreas for ptn3460 and ps8622. -- Split documentation patches from driver patches. I've tested your series on an Exynos5420 Peach Pit and an Exynos5250 Snow Chromebooks and display worked for me on both machines. Great! I also needed [PATCH] drm/panel: simple: Add AUO B116XW03 panel support [0] which does not apply cleanly on linux-next so you may want to do a re-spin for that patch. Ok. I will take care of this in next version. For Snow I also had to disable CONFIG_FB_SIMPLE, otherwise I just saw a blink on boot and only the backlight remained turned on (no display output). I don't know if that is expected since IIUC it should be possible to do a transition from simplefb to a DRM/KMS driver. I don't have a serial console hooked on this machine so I couldn't debug it further, sorry. I am just wondering how SIMPLE FB can affect DRM based display. I am not even sure if both can co-exist or not. Is there anything we can do with bootargs instead of CONFIG? I think the issue is that simplefb will register as console but the display driver is probably reconfiguring the display hardware and then registering a new console, so you'd end up with just a blank screen. Typically the simplefb device tree nodes will be added by the bootloader so you might be able to prevent the bootloader from adding it. Ideally, though, the solution would be to do a proper hand-off from simplefb to DRM/KMS so that you can have a seemless transition. Somewhere in the middle would be to make simplefb unload when loading the DRM/KMS driver. Thierry pgpvv6hhOLCNp.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Mon, Sep 22, 2014 at 02:01:38PM +0530, Ajay kumar wrote: Hi Thierry, On Mon, Sep 22, 2014 at 1:40 PM, Thierry Reding thierry.red...@gmail.com wrote: On Thu, Sep 18, 2014 at 11:20:40AM +0530, Ajay kumar wrote: Hi Tomi, On Wed, Sep 17, 2014 at 9:52 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 17/09/14 17:29, Ajay kumar wrote: Hi Tomi, Thanks for your comments. On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? use-external-pwm or disable-bridge-pwm would be better. Well, the properties are about the bridge. use-external-pwm means that the bridge uses an external PWM, which, if I understood correctly, is not what the property is about. disable-bridge-pwm is ok, but the bridge there is extra. The properties are about the bridge, so it's implicit. Ok. I will use disable-pwm. Why is this even necessary? According to the datasheet this device has circuitry for backlight control. If so, I'd expect it to expose either a backlight device or a PWM device. That way unless somebody is using the backlight/PWM exposed by the bridge the bridge can simply disable PWM. The driver does expose a backlight device. And, the decision(whether to expose a backlight device or not) is made based on the DT flag use-external-pwm. This was discussed before, and you suggested to use the boolean property, refer to this link: http://lists.freedesktop.org/archives/dri-devel/2014-July/065048.html I think you misunderstood what I said, or maybe I didn't explain clearly what I meant. If the PS8622 provides a backlight there's nothing wrong with always exposing it. The bridge itself isn't going to be using the backlight anyway. Rather the panel itself should be using the backlight device exposed by PS8622 or some separate backlight device. To illustrate by an example: ps8622: ... { compatible = parade,ps8622; ... }; panel { ... backlight = ps8622; ... }; Or: backlight: ... { compatible = pwm-backlight; ... }; panel { ... backlight = backlight; ... }; What you did in v6 of this series was look up a backlight device and then not use it. That seemed unnecessary. Looking at v6 again the reason for getting a phandle to the backlight was so that the device itself did not expose its own backlight controlling circuitry if an external one was being used. But since the bridge has no business controlling the backlight, having the backlight phandle in the bridge is not correct. So I think what you could do in the driver instead is always expose the backlight device. If the panel used a different backlight, the PS8622's internal on simply wouldn't be accessed. It would still be possible to control the backlight in sysfs, but that shouldn't be a problem (only root can access it). That said, I have no strong objections to the boolean property if you feel like it's really necessary. Thierry pgpEd4V9G6SyJ.pgp Description: PGP signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Mon, Sep 22, 2014 at 4:11 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 22, 2014 at 02:01:38PM +0530, Ajay kumar wrote: Hi Thierry, On Mon, Sep 22, 2014 at 1:40 PM, Thierry Reding thierry.red...@gmail.com wrote: On Thu, Sep 18, 2014 at 11:20:40AM +0530, Ajay kumar wrote: Hi Tomi, On Wed, Sep 17, 2014 at 9:52 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 17/09/14 17:29, Ajay kumar wrote: Hi Tomi, Thanks for your comments. On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? use-external-pwm or disable-bridge-pwm would be better. Well, the properties are about the bridge. use-external-pwm means that the bridge uses an external PWM, which, if I understood correctly, is not what the property is about. disable-bridge-pwm is ok, but the bridge there is extra. The properties are about the bridge, so it's implicit. Ok. I will use disable-pwm. Why is this even necessary? According to the datasheet this device has circuitry for backlight control. If so, I'd expect it to expose either a backlight device or a PWM device. That way unless somebody is using the backlight/PWM exposed by the bridge the bridge can simply disable PWM. The driver does expose a backlight device. And, the decision(whether to expose a backlight device or not) is made based on the DT flag use-external-pwm. This was discussed before, and you suggested to use the boolean property, refer to this link: http://lists.freedesktop.org/archives/dri-devel/2014-July/065048.html I think you misunderstood what I said, or maybe I didn't explain clearly what I meant. If the PS8622 provides a backlight there's nothing wrong with always exposing it. The bridge itself isn't going to be using the backlight anyway. Rather the panel itself should be using the backlight device exposed by PS8622 or some separate backlight device. To illustrate by an example: ps8622: ... { compatible = parade,ps8622; ... }; panel { ... backlight = ps8622; ... }; No, if ps8622 backlight control is used, we need not specify the backlight phandle for the panel driver. Somehow, ps8622 internal circuitry keeps the bootup glitch free :) Backlight control and panel controls can be separate then. Or: backlight: ... { compatible = pwm-backlight; ... }; panel { ... backlight = backlight; ... }; This is the way it is for peach_pit. What you did in v6 of this series was look up a backlight device and then not use it. That seemed unnecessary. Looking at v6 again the reason for getting a phandle to the backlight was so that the device itself did not expose its own backlight controlling circuitry if an external one was being used. But since the bridge has no business controlling the backlight, having the backlight phandle in the bridge is not correct. So I think what you could do in the driver instead is always expose the backlight device. If the panel used a different backlight, the PS8622's internal on simply wouldn't be accessed. It would still be possible to control the backlight in sysfs, but that shouldn't be a problem (only root can access it) That would be like simple exposing a feature which cannot be used by the user, ideally which should not be used by the user. That said, I have no strong objections to the boolean property if you feel like it's really necessary. Won't you think having a boolean property for an optional feature of
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Mon, Sep 22, 2014 at 04:53:22PM +0530, Ajay kumar wrote: On Mon, Sep 22, 2014 at 4:11 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 22, 2014 at 02:01:38PM +0530, Ajay kumar wrote: Hi Thierry, On Mon, Sep 22, 2014 at 1:40 PM, Thierry Reding thierry.red...@gmail.com wrote: On Thu, Sep 18, 2014 at 11:20:40AM +0530, Ajay kumar wrote: Hi Tomi, On Wed, Sep 17, 2014 at 9:52 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 17/09/14 17:29, Ajay kumar wrote: Hi Tomi, Thanks for your comments. On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? use-external-pwm or disable-bridge-pwm would be better. Well, the properties are about the bridge. use-external-pwm means that the bridge uses an external PWM, which, if I understood correctly, is not what the property is about. disable-bridge-pwm is ok, but the bridge there is extra. The properties are about the bridge, so it's implicit. Ok. I will use disable-pwm. Why is this even necessary? According to the datasheet this device has circuitry for backlight control. If so, I'd expect it to expose either a backlight device or a PWM device. That way unless somebody is using the backlight/PWM exposed by the bridge the bridge can simply disable PWM. The driver does expose a backlight device. And, the decision(whether to expose a backlight device or not) is made based on the DT flag use-external-pwm. This was discussed before, and you suggested to use the boolean property, refer to this link: http://lists.freedesktop.org/archives/dri-devel/2014-July/065048.html I think you misunderstood what I said, or maybe I didn't explain clearly what I meant. If the PS8622 provides a backlight there's nothing wrong with always exposing it. The bridge itself isn't going to be using the backlight anyway. Rather the panel itself should be using the backlight device exposed by PS8622 or some separate backlight device. To illustrate by an example: ps8622: ... { compatible = parade,ps8622; ... }; panel { ... backlight = ps8622; ... }; No, if ps8622 backlight control is used, we need not specify the backlight phandle for the panel driver. Somehow, ps8622 internal circuitry keeps the bootup glitch free :) Backlight control and panel controls can be separate then. But they shouldn't. Your panel driver should always be the one to control backlight. How else is the bridge supposed to know when to turn backlight on or off? What you did in v6 of this series was look up a backlight device and then not use it. That seemed unnecessary. Looking at v6 again the reason for getting a phandle to the backlight was so that the device itself did not expose its own backlight controlling circuitry if an external one was being used. But since the bridge has no business controlling the backlight, having the backlight phandle in the bridge is not correct. So I think what you could do in the driver instead is always expose the backlight device. If the panel used a different backlight, the PS8622's internal on simply wouldn't be accessed. It would still be possible to control the backlight in sysfs, but that shouldn't be a problem (only root can access it) That would be like simple exposing a feature which cannot be used by the user, ideally which should not be used by the user. And it won't be used unless they access the sysfs files directly. There are a lot of cases
[PATCH] ARM: dts: s3c64xx: Enable PWM node by default
The PWM block is required for system clock source so it must be always enabled. This patch fixes boot issues on SMDK6410 which did not have the node enabled explicitly for other purposes. Fixes: eeb93d02c5d8 (clocksource: of: Respect device tree node status) Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- arch/arm/boot/dts/s3c6410-mini6410.dts | 4 arch/arm/boot/dts/s3c64xx.dtsi | 1 - 2 files changed, 5 deletions(-) diff --git a/arch/arm/boot/dts/s3c6410-mini6410.dts b/arch/arm/boot/dts/s3c6410-mini6410.dts index a171cdd26b7f..42c2bcdf2626 100644 --- a/arch/arm/boot/dts/s3c6410-mini6410.dts +++ b/arch/arm/boot/dts/s3c6410-mini6410.dts @@ -252,10 +252,6 @@ status = okay; }; -pwm { - status = okay; -}; - pinctrl0 { gpio_leds: gpio-leds { samsung,pins = gpk-4, gpk-5, gpk-6, gpk-7; diff --git a/arch/arm/boot/dts/s3c64xx.dtsi b/arch/arm/boot/dts/s3c64xx.dtsi index 05c717ad1edd..196fe6ea205d 100644 --- a/arch/arm/boot/dts/s3c64xx.dtsi +++ b/arch/arm/boot/dts/s3c64xx.dtsi @@ -239,7 +239,6 @@ clocks = clocks PCLK_PWM; samsung,pwm-outputs = 0, 1; #pwm-cells = 3; - status = disabled; }; pinctrl0: pinctrl@7f008000 { -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ARM: dts: s3c64xx: Enable PWM node by default
Tomasz Figa wrote: The PWM block is required for system clock source so it must be always enabled. This patch fixes boot issues on SMDK6410 which did not have the node enabled explicitly for other purposes. Fixes: eeb93d02c5d8 (clocksource: of: Respect device tree node status) Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- arch/arm/boot/dts/s3c6410-mini6410.dts | 4 arch/arm/boot/dts/s3c64xx.dtsi | 1 - 2 files changed, 5 deletions(-) diff --git a/arch/arm/boot/dts/s3c6410-mini6410.dts b/arch/arm/boot/dts/s3c6410-mini6410.dts index a171cdd26b7f..42c2bcdf2626 100644 --- a/arch/arm/boot/dts/s3c6410-mini6410.dts +++ b/arch/arm/boot/dts/s3c6410-mini6410.dts @@ -252,10 +252,6 @@ status = okay; }; -pwm { - status = okay; -}; - pinctrl0 { gpio_leds: gpio-leds { samsung,pins = gpk-4, gpk-5, gpk-6, gpk-7; diff --git a/arch/arm/boot/dts/s3c64xx.dtsi b/arch/arm/boot/dts/s3c64xx.dtsi index 05c717ad1edd..196fe6ea205d 100644 --- a/arch/arm/boot/dts/s3c64xx.dtsi +++ b/arch/arm/boot/dts/s3c64xx.dtsi @@ -239,7 +239,6 @@ clocks = clocks PCLK_PWM; samsung,pwm-outputs = 0, 1; #pwm-cells = 3; - status = disabled; }; pinctrl0: pinctrl@7f008000 { -- 2.1.0 Looks good, will apply. Thanks, Kukjin -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] ARM: dts: Add rtc_src clk for s3c-rtc on exynos5250-snow
Andreas Färber wrote: Am 17.09.2014 um 17:47 schrieb Doug Anderson: Hi, On Wed, Sep 17, 2014 at 4:50 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: commit 546b117fdf17 (rtc: s3c: add support for RTC of Exynos3250 SoC) added an rtc_src DT property for the Samsung's S3C Real Time Clock controller that specifies the 32.768 kHz clock that uses the RTC as its source clock. In the case of the Exynos5250 based Snow board, the Maxim 77686 32kHz AP clock is used as the source clock. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5250-snow.dts | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts index e51fcef..56eec56 100644 --- a/arch/arm/boot/dts/exynos5250-snow.dts +++ b/arch/arm/boot/dts/exynos5250-snow.dts @@ -10,6 +10,7 @@ /dts-v1/; #include dt-bindings/gpio/gpio.h +#include dt-bindings/clock/maxim,max77686.h #include exynos5250.dtsi / { @@ -29,6 +30,8 @@ rtc@101E { status = okay; + clocks = clock CLK_RTC, max77686 MAX77686_CLK_AP; + clock-names = rtc, rtc_src; Wait, seriously? Snow is still using the rtc@101E syntax? Whatever happened to the series that Andreas worked so hard on, including https://patchwork.kernel.org/patch/4664801/? Kukjin: Andreas's patch series was Reviewed long ago I think and by now I'd imagine it's got some conflicts due to it not having been applied in a timely fashion. Perhaps you could fix it up for Andreas (since he's already rebased it several times) and land it? If I had to guess, outstanding from Andreas's series (patchwork IDs listed): 4751131 New [v7] ARM: dts: Prepare node labels for exynos5250 4664801 New [v6,04/10] ARM: dts: Clean up exynos5250-snow 4664771 New [v6,05/10] ARM: dts: Fill in bootargs for exynos5250-snow 4664731 New [v6,06/10] ARM: dts: Clean up exynos5250-smdk5250 4664751 New [v6,07/10] ARM: dts: Clean up exynos5250-arndale 4664711 New [v6,08/10] ARM: dts: Fix apparent GPIO typo in exynos5250-arndale 4664681 New [v6,09/10] ARM: dts: Simplify USB3503 on exynos5250-arndale 4664691 New [v6,10/10] ARM: dts: Add exynos5250-spring device tree Yes, Kukjin told me not to resend anything, and I haven't heard back from him since his Kernel Summit trip. Sorry about that and I thought I took them :( I'll sort them out tonight including this series together. - Kukjin -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/18] clk: exynos: add missing smmu_g2d clock and update comments
On 16.09.2014 13:54, Marek Szyprowski wrote: This patch adds missing smmu_g2d clock implementation and updates comment about Exynos4 clocks from 278-282 range. Those clocks are available on all Exynos4 SoC series, so the misleading comment has been removed. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Acked-by: Tomasz Figa t.f...@samsung.com --- drivers/clk/samsung/clk-exynos4.c | 1 + include/dt-bindings/clock/exynos4.h | 10 +- 2 files changed, 6 insertions(+), 5 deletions(-) Applied for next. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Mon, Sep 22, 2014 at 5:05 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 22, 2014 at 04:53:22PM +0530, Ajay kumar wrote: On Mon, Sep 22, 2014 at 4:11 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 22, 2014 at 02:01:38PM +0530, Ajay kumar wrote: Hi Thierry, On Mon, Sep 22, 2014 at 1:40 PM, Thierry Reding thierry.red...@gmail.com wrote: On Thu, Sep 18, 2014 at 11:20:40AM +0530, Ajay kumar wrote: Hi Tomi, On Wed, Sep 17, 2014 at 9:52 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 17/09/14 17:29, Ajay kumar wrote: Hi Tomi, Thanks for your comments. On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? use-external-pwm or disable-bridge-pwm would be better. Well, the properties are about the bridge. use-external-pwm means that the bridge uses an external PWM, which, if I understood correctly, is not what the property is about. disable-bridge-pwm is ok, but the bridge there is extra. The properties are about the bridge, so it's implicit. Ok. I will use disable-pwm. Why is this even necessary? According to the datasheet this device has circuitry for backlight control. If so, I'd expect it to expose either a backlight device or a PWM device. That way unless somebody is using the backlight/PWM exposed by the bridge the bridge can simply disable PWM. The driver does expose a backlight device. And, the decision(whether to expose a backlight device or not) is made based on the DT flag use-external-pwm. This was discussed before, and you suggested to use the boolean property, refer to this link: http://lists.freedesktop.org/archives/dri-devel/2014-July/065048.html I think you misunderstood what I said, or maybe I didn't explain clearly what I meant. If the PS8622 provides a backlight there's nothing wrong with always exposing it. The bridge itself isn't going to be using the backlight anyway. Rather the panel itself should be using the backlight device exposed by PS8622 or some separate backlight device. To illustrate by an example: ps8622: ... { compatible = parade,ps8622; ... }; panel { ... backlight = ps8622; ... }; No, if ps8622 backlight control is used, we need not specify the backlight phandle for the panel driver. Somehow, ps8622 internal circuitry keeps the bootup glitch free :) Backlight control and panel controls can be separate then. But they shouldn't. Your panel driver should always be the one to control backlight. How else is the bridge supposed to know when to turn backlight on or off? In internal pwm case, we keep the backlight on in probe, and from userspace its upto the user to control it via sysfs. And, ps8622 generates backlight only if video data is coming from the encoder. Backlight is synced with video data using an internal circuit, I think. Since internal pwm is tied to video data, but not to any of the panel controls, we need not do any backlight control in panel driver. What you did in v6 of this series was look up a backlight device and then not use it. That seemed unnecessary. Looking at v6 again the reason for getting a phandle to the backlight was so that the device itself did not expose its own backlight controlling circuitry if an external one was being used. But since the bridge has no business controlling the backlight, having the backlight phandle in the bridge is not correct. So I think what you could do in the driver
[PATCH] clk: exynos4: fix g3d clocks
sclk_g3d clock doesn't have enable/disable bits, but the driver hijacked g3d gate clock bits for this purpose and didn't provide real g3d clock at all. This patch fixes this issue by adding proper definition for g3d clock and removing incorrect access to GATE_IP_G3D register in sclk_g3d. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- drivers/clk/samsung/clk-exynos4.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c index ac163d7f5bc3..111be8469e3d 100644 --- a/drivers/clk/samsung/clk-exynos4.c +++ b/drivers/clk/samsung/clk-exynos4.c @@ -733,8 +733,7 @@ static struct samsung_div_clock exynos4_div_clks[] __initdata = { DIV(0, div_csis0, mout_csis0, DIV_CAM, 24, 4), DIV(0, div_csis1, mout_csis1, DIV_CAM, 28, 4), DIV(CLK_SCLK_MFC, sclk_mfc, mout_mfc, DIV_MFC, 0, 4), - DIV_F(0, div_g3d, mout_g3d, DIV_G3D, 0, 4, - CLK_SET_RATE_PARENT, 0), + DIV(CLK_SCLK_G3D, sclk_g3d, mout_g3d, DIV_G3D, 0, 4), DIV(0, div_fimd0, mout_fimd0, DIV_LCD0, 0, 4), DIV(0, div_mipi0, mout_mipi0, DIV_LCD0, 16, 4), DIV(0, div_audio0, mout_audio0, DIV_MAUDIO, 0, 4), @@ -857,8 +856,7 @@ static struct samsung_gate_clock exynos4_gate_clks[] __initdata = { 0), GATE(CLK_TSI, tsi, aclk133, GATE_IP_FSYS, 4, 0, 0), GATE(CLK_SROMC, sromc, aclk133, GATE_IP_FSYS, 11, 0, 0), - GATE(CLK_SCLK_G3D, sclk_g3d, div_g3d, GATE_IP_G3D, 0, - CLK_SET_RATE_PARENT, 0), + GATE(CLK_G3D, g3d, aclk200, GATE_IP_G3D, 0, 0, 0), GATE(CLK_PPMUG3D, ppmug3d, aclk200, GATE_IP_G3D, 1, 0, 0), GATE(CLK_USB_DEVICE, usb_device, aclk133, GATE_IP_FSYS, 13, 0, 0), GATE(CLK_ONENAND, onenand, aclk133, GATE_IP_FSYS, 15, 0, 0), -- 1.9.2 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clk: exynos4: fix g3d clocks
On 22.09.2014 14:17, Marek Szyprowski wrote: sclk_g3d clock doesn't have enable/disable bits, but the driver hijacked g3d gate clock bits for this purpose and didn't provide real g3d clock at all. This patch fixes this issue by adding proper definition for g3d clock and removing incorrect access to GATE_IP_G3D register in sclk_g3d. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- drivers/clk/samsung/clk-exynos4.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) Applied for next with a minor adjustment of patch description. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] s5p-mfc: Use decode status instead of display status on MFCv5
Commit 90c0ae50097 changed how the frame_type of a decoded frame gets determined, by switching from the get_dec_frame_type to get_disp_frame_type operation. Unfortunately it seems that on MFC v5 the result of get_disp_frame_type is always 0 (no display) when decoding (tested with H264), resulting in no frame ever being output from the decoder. This patch reverts MFC v5 to the previous behaviour while keeping the new behaviour for v6 and up. Signed-off-by: Sjoerd Simons sjoerd.sim...@collabora.co.uk --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index d35b041..27ca9d0 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -264,7 +264,12 @@ static void s5p_mfc_handle_frame_new(struct s5p_mfc_ctx *ctx, unsigned int err) unsigned int frame_type; dspl_y_addr = s5p_mfc_hw_call(dev-mfc_ops, get_dspl_y_adr, dev); - frame_type = s5p_mfc_hw_call(dev-mfc_ops, get_disp_frame_type, ctx); + if (IS_MFCV6_PLUS(dev)) + frame_type = s5p_mfc_hw_call(dev-mfc_ops, + get_disp_frame_type, ctx); + else + frame_type = s5p_mfc_hw_call(dev-mfc_ops, + get_dec_frame_type, dev); /* If frame is same as previous then skip and do not dequeue */ if (frame_type == S5P_FIMV_DECODE_FRAME_SKIPPED) { -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 0/2] serial: samsung: add support for early console
Hello, This patchset adds support for early console defined in device tree. As an example, DTS files for all Exynos4 based machines are updated with the correct value for common chosen/sdtout property. To get it fully functional additional improvements (support for early_ioremap) are needed in early console code. Best regards Marek Szyprowski Samsung RD Institute Poland Patch summary: Tomasz Figa (2): serial: samsung: Add support for of_earlycon ARM: dts: exynos4: Add stdout-path properties arch/arm/boot/dts/exynos4210-origen.dts | 1 + arch/arm/boot/dts/exynos4210-smdkv310.dts | 1 + arch/arm/boot/dts/exynos4210-trats.dts | 1 + arch/arm/boot/dts/exynos4210-universal_c210.dts | 1 + arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 4 + arch/arm/boot/dts/exynos4412-origen.dts | 1 + arch/arm/boot/dts/exynos4412-smdk4412.dts | 1 + arch/arm/boot/dts/exynos4412-tiny4412.dts | 4 + arch/arm/boot/dts/exynos4412-trats2.dts | 1 + drivers/tty/serial/Kconfig | 1 + drivers/tty/serial/samsung.c| 97 + 11 files changed, 113 insertions(+) -- 1.9.2 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 1/2] serial: samsung: Add support for of_earlycon
From: Tomasz Figa t.f...@samsung.com This patch adds support for early console initialized from device tree to all variants of Samsung serial driver. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- drivers/tty/serial/Kconfig | 1 + drivers/tty/serial/samsung.c | 97 2 files changed, 98 insertions(+) diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index 26cec64dadd7..e87a2dfd9080 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -222,6 +222,7 @@ config SERIAL_SAMSUNG tristate Samsung SoC serial support depends on PLAT_SAMSUNG select SERIAL_CORE + select SERIAL_EARLYCON help Support for the on-chip UARTs on the Samsung S3C24XX series CPUs, providing /dev/ttySAC0, 1 and 2 (note, some machines may not diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c index c78f43a481ce..71829ce2410d 100644 --- a/drivers/tty/serial/samsung.c +++ b/drivers/tty/serial/samsung.c @@ -1856,6 +1856,103 @@ static struct platform_driver samsung_serial_driver = { module_platform_driver(samsung_serial_driver); +/* + * Early console. + */ + +struct samsung_early_console_data { + u32 txfull_mask; +}; + +static void samsung_early_busyuart(struct uart_port *port) +{ + while (!(readl(port-membase + S3C2410_UTRSTAT) S3C2410_UTRSTAT_TXFE)) + ; +} + +static void samsung_early_busyuart_fifo(struct uart_port *port) +{ + struct samsung_early_console_data *data = port-private_data; + + while (readl(port-membase + S3C2410_UFSTAT) data-txfull_mask) + ; +} + +static void samsung_early_putc(struct uart_port *port, int c) +{ + if (readl(port-membase + S3C2410_UFCON) S3C2410_UFCON_FIFOMODE) + samsung_early_busyuart_fifo(port); + else + samsung_early_busyuart(port); + + writeb(c, port-membase + S3C2410_UTXH); +} + +static void samsung_early_write(struct console *con, const char *s, unsigned n) +{ + struct earlycon_device *dev = con-data; + + uart_console_write(dev-port, s, n, samsung_early_putc); +} + +static int __init samsung_early_console_setup(struct earlycon_device *device, + const char *opt) +{ + if (!device-port.membase) + return -ENODEV; + + device-con-write = samsung_early_write; + return 0; +} + +/* S3C2410, S3C2412 */ +static struct samsung_early_console_data s3c2410_early_console_data = { + .txfull_mask = S3C2410_UFSTAT_TXFULL, +}; + +static int __init s3c2410_early_console_setup(struct earlycon_device *device, + const char *opt) +{ + device-port.private_data = s3c2410_early_console_data; + return samsung_early_console_setup(device, opt); +} +OF_EARLYCON_DECLARE(s3c2410, samsung,s3c2410-uart, + s3c2410_early_console_setup); + +/* S3C2440, S3C64xx */ +static struct samsung_early_console_data s3c2440_early_console_data = { + .txfull_mask = S3C2440_UFSTAT_TXFULL, +}; + +static int __init s3c2440_early_console_setup(struct earlycon_device *device, + const char *opt) +{ + device-port.private_data = s3c2440_early_console_data; + return samsung_early_console_setup(device, opt); +} +OF_EARLYCON_DECLARE(s3c2412, samsung,s3c2412-uart, + s3c2440_early_console_setup); +OF_EARLYCON_DECLARE(s3c2440, samsung,s3c2440-uart, + s3c2440_early_console_setup); +OF_EARLYCON_DECLARE(s3c6400, samsung,s3c6400-uart, + s3c2440_early_console_setup); + +/* S5PV210, EXYNOS */ +static struct samsung_early_console_data s5pv210_early_console_data = { + .txfull_mask = S5PV210_UFSTAT_TXFULL, +}; + +static int __init s5pv210_early_console_setup(struct earlycon_device *device, + const char *opt) +{ + device-port.private_data = s5pv210_early_console_data; + return samsung_early_console_setup(device, opt); +} +OF_EARLYCON_DECLARE(s5pv210, samsung,s5pv210-uart, + s5pv210_early_console_setup); +OF_EARLYCON_DECLARE(exynos4210, samsung,exynos4210-uart, + s5pv210_early_console_setup); + MODULE_ALIAS(platform:samsung-uart); MODULE_DESCRIPTION(Samsung SoC Serial port driver); MODULE_AUTHOR(Ben Dooks b...@simtec.co.uk); -- 1.9.2 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 2/2] ARM: dts: exynos4: Add stdout-path properties
From: Tomasz Figa t.f...@samsung.com This patch adds stdout-path property to chosen nodes of Exynos4 boards to enable use of earlycon feature without the need to hardcode port number in kernel itself. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- arch/arm/boot/dts/exynos4210-origen.dts | 1 + arch/arm/boot/dts/exynos4210-smdkv310.dts | 1 + arch/arm/boot/dts/exynos4210-trats.dts | 1 + arch/arm/boot/dts/exynos4210-universal_c210.dts | 1 + arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 4 arch/arm/boot/dts/exynos4412-origen.dts | 1 + arch/arm/boot/dts/exynos4412-smdk4412.dts | 1 + arch/arm/boot/dts/exynos4412-tiny4412.dts | 4 arch/arm/boot/dts/exynos4412-trats2.dts | 1 + 9 files changed, 15 insertions(+) diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts index f767c425d0b5..b81146141402 100644 --- a/arch/arm/boot/dts/exynos4210-origen.dts +++ b/arch/arm/boot/dts/exynos4210-origen.dts @@ -31,6 +31,7 @@ chosen { bootargs =root=/dev/ram0 rw ramdisk=8192 initrd=0x4100,8M console=ttySAC2,115200 init=/linuxrc; + stdout-path = serial_2; }; regulators { diff --git a/arch/arm/boot/dts/exynos4210-smdkv310.dts b/arch/arm/boot/dts/exynos4210-smdkv310.dts index 676e6e0c8cf3..86216fff1b4f 100644 --- a/arch/arm/boot/dts/exynos4210-smdkv310.dts +++ b/arch/arm/boot/dts/exynos4210-smdkv310.dts @@ -27,6 +27,7 @@ chosen { bootargs = root=/dev/ram0 rw ramdisk=8192 initrd=0x4100,8M console=ttySAC1,115200 init=/linuxrc; + stdout-path = serial_1; }; sdhci@1253 { diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts index f516da9e8b3a..b351c7bddf2d 100644 --- a/arch/arm/boot/dts/exynos4210-trats.dts +++ b/arch/arm/boot/dts/exynos4210-trats.dts @@ -28,6 +28,7 @@ chosen { bootargs = console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rootwait earlyprintk panic=5; + stdout-path = serial_2; }; regulators { diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts b/arch/arm/boot/dts/exynos4210-universal_c210.dts index d50eb3aa708e..e65ee3cb36c3 100644 --- a/arch/arm/boot/dts/exynos4210-universal_c210.dts +++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts @@ -26,6 +26,7 @@ chosen { bootargs = console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw rootwait earlyprintk panic=5 maxcpus=1; + stdout-path = serial_2; }; sysram@0202 { diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi index adadaf97ac01..0841fd34f9dd 100644 --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi @@ -11,6 +11,10 @@ #include exynos4412.dtsi / { + chosen { + stdout-path = serial_1; + }; + firmware@0204F000 { compatible = samsung,secure-firmware; reg = 0x0204F000 0x1000; diff --git a/arch/arm/boot/dts/exynos4412-origen.dts b/arch/arm/boot/dts/exynos4412-origen.dts index e925c9fbfb07..979dc07c929c 100644 --- a/arch/arm/boot/dts/exynos4412-origen.dts +++ b/arch/arm/boot/dts/exynos4412-origen.dts @@ -26,6 +26,7 @@ chosen { bootargs =console=ttySAC2,115200; + stdout-path = serial_2; }; firmware@0203F000 { diff --git a/arch/arm/boot/dts/exynos4412-smdk4412.dts b/arch/arm/boot/dts/exynos4412-smdk4412.dts index ded0b70f7644..b9256afbcc68 100644 --- a/arch/arm/boot/dts/exynos4412-smdk4412.dts +++ b/arch/arm/boot/dts/exynos4412-smdk4412.dts @@ -25,6 +25,7 @@ chosen { bootargs =root=/dev/ram0 rw ramdisk=8192 initrd=0x4100,8M console=ttySAC1,115200 init=/linuxrc; + stdout-path = serial_1; }; g2d@1080 { diff --git a/arch/arm/boot/dts/exynos4412-tiny4412.dts b/arch/arm/boot/dts/exynos4412-tiny4412.dts index ea6929d9c621..d46fd4c2aeaa 100644 --- a/arch/arm/boot/dts/exynos4412-tiny4412.dts +++ b/arch/arm/boot/dts/exynos4412-tiny4412.dts @@ -18,6 +18,10 @@ model = FriendlyARM TINY4412 board based on Exynos4412; compatible = friendlyarm,tiny4412, samsung,exynos4412, samsung,exynos4; + chosen { + stdout-path = serial_0; + }; + memory { reg = 0x4000 0x4000; }; diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts index 11967f4561e0..ade31d3174b0 100644 --- a/arch/arm/boot/dts/exynos4412-trats2.dts +++ b/arch/arm/boot/dts/exynos4412-trats2.dts @@ -30,6 +30,7 @@ chosen { bootargs = console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rootwait earlyprintk panic=5; +
Re: [PATCH] tty/serial: samsung: Add earlycon support
Hi Tomasz, On Mon, Sep 22, 2014 at 4:49 AM, Tomasz Figa tomasz.f...@gmail.com wrote: On 22.09.2014 01:10, Alim Akhtar wrote: [snip] As you said there is no support for ioremap on ARM, so this is not tested on ARM. Don't forget that this driver is primarily targeted for ARM platforms (versus just one ARM64-based Exynos7), so either this feature should be clearly added as ARM64-specific (and compiled in conditionally) or made work for all supported platforms. Well, this will work on every platform which uses samsung,exynos4210-uart as a UART controller. Exynos7 also use same, there is nothing special about ARM64 bit here. please see[1]. For s3c24xx-uart, this has to be implemented separably as that is a bit different. For this feature, they differ only in locations and lengths of FIFO-related bit fields. We had this already implemented for all hardware variants and so my recommendation to reuse that code. smh Use ARM semihosting calls for early console. earlyprintk=[X86,SH,BLACKFIN,ARM,M68k] [snip] Tomasz/Marek, Do you think something like below make sense here? +static void exynos4210_serial_console_putc(struct uart_port *port, int ch) +{ + while (!(readl(port-membase + S3C2410_UFCON) S3C2410_UFCON_FIFOMODE)) + ; Waiting in a loop for a software-writable FIFO mode enable bit doesn't look reasonable to me. Probably a typo? + + wr_regb(port, S3C2410_UTXH, ch); + + while ((readl(port-membase + S3C2410_UFSTAT) S5PV210_UFSTAT_TXFULL)) + ; I wonder if you need to wait for the character to be sent. If you ensure before writing a character that there is no previous one in the buffer or there is space in FIFO then I believe you should be fine. Well, below is what I was about to post: #define S3C24XX_SERIAL_CONSOLE s3c24xx_serial_console +static void s3c24xx_serial_console_putc(struct uart_port *port, int ch) +{ + unsigned int ufcon = rd_regl(port, S3C2410_UFCON); + + if (ufcon S3C2410_UFCON_FIFOMODE) { + while((readl(port-membase + S3C2410_UFSTAT) + S5PV210_UFSTAT_TXFULL)) + ; + wr_regb(port, S3C2410_UTXH, ch); + } +} + +static void samsung_early_write(struct console *con, const char *s, unsigned n) +{ + struct earlycon_device *dev = con-data; + + uart_console_write(dev-port, s, n, s3c24xx_serial_console_putc); +} + +static int __init samsung_early_console_setup(struct earlycon_device *device, + const char *opt) +{ + if (!device-port.membase) + return -ENODEV; + device-con-write = samsung_early_write; + return 0; +} +EARLYCON_DECLARE(samsung, samsung_early_console_setup); +OF_EARLYCON_DECLARE(samsung, samsung,exynos4210-uart, samsung_early_console_setup); but just saw patches from Marek few min back which is similar except for the handle non-fifo mode, which is good anyway. :-) So lets drops this patch and discussion here. Thanks!!! +} and call exynos4210_serial_console_putc() in samsung_early_write()? Anyway functions names need to be changes accordingly. Yes, this is exactly what we had implemented in the patch I mentioned, except that the putc function was generic for all hardware variants. Best regards, Tomasz -- Regards, Alim -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 22/09/14 10:54, Thierry Reding wrote: I wish all new display component bindings would use the video ports/endpoints to describe the connections. It will be very difficult to improve the display driver model later if we're missing such critical pieces from the DT bindings. I disagree. Why would we want to burden all devices with a bloated I agree that the .dts becomes more bloated with video graph. binding and drivers with parsing a complex graph when it's not even Hopefully not all drivers will need to do the parsing themselves. If it's common stuff, I don't see why we can't have helpers to do the work. known that it will be necessary? Evidently this device works fine using the current binding. Just because there are bindings to describe Well, I can write almost any kind of bindings, and then evidently my device would work. For me, on my board. ports in a generic way doesn't mean it has to be applied everywhere. After all the concept of ports/endpoints applies to non-video devices too, yet we don't require the bindings for those devices to add ports or endpoints nodes. I guess non-video devices haven't had need for those. I have had lots of boards with video setup that cannot be represented with simple phandles. I'm not sure if I have just been unlucky or what, but my understand is that other people have encountered such boards also. Usually the problems encountered there have been circumvented with some hacky video driver for that specific board, or maybe a static configuration handled by the boot loader. Also it won't be very difficult to extend the binding in a backwards compatible way if that becomes necessary. I don't know about that. More or less all the cases I've encountered where a binding has not been good from the start have been all but not very difficult. Do we have a standard way of representing the video pipeline with simple phandles? Or does everyone just do their own version? If there's no standard way, it sounds it'll be a mess to support in the future. If there's much objection to the bloatiness of video graphs, maybe we could come up with an simpler alternative (like the phandles here), and standardize it. Then, if common display framework or some such ever realizes, we could say that if your driver uses CDF, you need to support these video graph bindings and these simpler bindings to be compatible. However, I do have a slight recollection that alternative simplifications to the video graph were discussed at some point, and, while I may remember wrong, I think it was not accepted. At least I had one method to simplify the ports/endpoints, but that was removed from the patches I had. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 22/09/14 11:06, Thierry Reding wrote: Why do we need a complex graph when it can be handled using a simple phandle? Maybe in your case you can handle it with simple phandle. Can you guarantee that it's enough for everyone, on all platforms? Nobody can guarantee that. An interesting effect that DT ABI stability has had is that people now try to come up with overly generic concepts to describe devices in an attempt to make them more future-proof. I don't think we're doing ourselves a favour with that approach. I kind of agree. However, there are boards that need a more complex approach than just a simple phandle. They are nothing new. So while it may be true that this specific encoder has never been used in such a complex way, and there's a chance that it never will, my comments are not really about this particular encoder. What I want is a standard way to describe the video components. If we have a standard complex one (video graphs) and a standard simple one (simple phandles), it's ok for me. The point of the ports/endpoint graph is to also support more complicated scenarios. But the scenario will always remain the same for this bridge. There will always be an input signal and a translation to some output signal along with some parameters to control how that translation happens. More complicated scenarios will likely need to be represented differently at a higher level than the bridge. Yes, there's always one active input and one output for this bridge. What the video graphs would bring is to have the possibility to have multiple inputs and outputs, of which a single ones could be active at a time. The different inputs and outputs could even have different settings required (say, first output requires this bit set, but when using second output that bit must be cleared). If you now create ps8622 bindings that do not support those graphs, the no one else using ps8622 can use ports/endpoint graphs either. That's not true. The binding can easily be extended in a backwards- compatible way later on (should it really prove necessary). I hope you are right =). As I said in my earlier mail, my experience so far has been different. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 22/09/14 11:26, Thierry Reding wrote: On Fri, Sep 19, 2014 at 05:28:37PM +0300, Tomi Valkeinen wrote: On 19/09/14 16:59, Ajay kumar wrote: I am not really able to understand, what's stopping us from using this bridge on a board with complex display connections. To use ps8622 driver, one needs to attach it to the DRM framework. For this, the DRM driver Remember that when we talk about DT bindings, there's no such thing as DRM. We talk about hardware. The same bindings need to work on any operating system. would need the DT node for ps8622 bridge. For which I use a phandle. A complex one could be for example a case where you have two different panels connected to ps8622, and you can switch between the two panels with, say, a gpio. How do you present that with a simple phandle? How do you represent that with a graph? Whether you describe it using a graph or a simple phandle you'll need additional nodes somewhere in between. Perhaps something like this: mux: ... { compatible = gpio-mux-bridge; gpio = gpio 42 GPIO_ACTIVE_HIGH; panel@0 { panel = panel0; }; panel@1 { panel = panel1; }; }; ps8622: ... { bridge = mux; }; panel0: ... { ... }; panel1: ... { ... }; Yes, it's true we need a mux there. But we still have the complication that for panel0 we may need different ps8622 settings than for panel1. If that's the case, then I think we'd need to have two output endpoints in ps8622, both going to the mux, and each having the settings for the respective panel. If some XYZ platform wishes to pick the DT node via a different method, they are always welcome to do it. Just because I am not specifying a video port/endpoint in the DT binding example, would it mean that platform cannot make use of ports in future? If that is the case, I can add something All the platforms share the same bindings for ps8622. If you now specify that ps8622 bindings use a simple phandle, then anyone who uses ps8622 should support that. Of course the bindings can be extended in the future. In that case the drivers need to support both the old and the new bindings, which is always a hassle. Generally speaking, I sense that we have different views of how display devices and drivers are structured. You say If some XYZ platform wishes to pick the DT node via a different method, they are always welcome to do it.. This sounds to me that you see the connections between display devices as something handled by a platform specific driver. I, on the other hand, see connections between display devices as common properties. Say, we could have a display board, with a panel and an encoder and maybe some other components, which takes parallel RGB as input. The same display board could as well be connected to an OMAP board or to an Exynos board. I think the exact same display-board.dtsi file, which describes the devices and connections in the display board, should be usable on both OMAP and Exynos platforms. This means we need to have a common way to describe video devices, just as we have for other things. A common way to describe devices in DT isn't going to give you that. The device tree is completely missing any information about how to access an extension board like that. The operating system defines the API by which the board can be accessed. I imagine that this would work by making the first component of the board a bridge of some sort and then every driver that supports that type of bridge (ideally just a generic drm_bridge) would also work with that display board. I'm not sure I follow. Obviously there needs to be board specific .dts file that brings the board and the display board together. So, say, the display-board.dtsi has a i2c touchscreen node, but the board.dts will tell which i2c bus it's connected to. Well, now as I wrote that, I wonder if that's possible... A node needs to have a parent, and for i2c that must be the i2c master. Is that something the DT overlays/fragments or such are supposed to handle? But let's only think about the video path for now. We could have an encoder and a panel on the board. We could describe the video path between the encoder and the panel in the display-board.dts as that is fixed. Then all that's needed in the board.dts is to connect the board's video output to the encoders input with the video graph. Why would that not work? Sure, there's more that's needed. Common encoder and panel drivers for one. But it all starts with a common way to describe the video devices and the connections in the DT. If we don't have that, we don't have anything. Whether this is described using a single phandle to the bridge or a more complicated graph is irrelevant. What matters is that you get a phandle to the
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On 20/09/14 18:27, Javier Martinez Canillas wrote: I see that Documentation/devicetree/bindings/video/ti,omap-dss.txt mentions that the Video Ports binding documentation is in Documentation/devicetree/bindings/video/video-ports.txt but I don't see that this file exists in the kernel [1]. I see though that is was included on your series adding DSS DT support [2]. Do you know what happened with this file? Ah, I need to update the link. This describes the graph: Documentation/devicetree/bindings/graph.txt Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Monday 22 September 2014 13:35:15 Thierry Reding wrote: On Mon, Sep 22, 2014 at 04:53:22PM +0530, Ajay kumar wrote: On Mon, Sep 22, 2014 at 4:11 PM, Thierry Reding wrote: On Mon, Sep 22, 2014 at 02:01:38PM +0530, Ajay kumar wrote: On Mon, Sep 22, 2014 at 1:40 PM, Thierry Reding wrote: On Thu, Sep 18, 2014 at 11:20:40AM +0530, Ajay kumar wrote: On Wed, Sep 17, 2014 at 9:52 PM, Tomi Valkeinen wrote: On 17/09/14 17:29, Ajay kumar wrote: On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? use-external-pwm or disable-bridge-pwm would be better. Well, the properties are about the bridge. use-external-pwm means that the bridge uses an external PWM, which, if I understood correctly, is not what the property is about. disable-bridge-pwm is ok, but the bridge there is extra. The properties are about the bridge, so it's implicit. Ok. I will use disable-pwm. Why is this even necessary? According to the datasheet this device has circuitry for backlight control. If so, I'd expect it to expose either a backlight device or a PWM device. That way unless somebody is using the backlight/PWM exposed by the bridge the bridge can simply disable PWM. The driver does expose a backlight device. And, the decision(whether to expose a backlight device or not) is made based on the DT flag use-external-pwm. This was discussed before, and you suggested to use the boolean property, refer to this link: http://lists.freedesktop.org/archives/dri-devel/2014-July/065048.html I think you misunderstood what I said, or maybe I didn't explain clearly what I meant. If the PS8622 provides a backlight there's nothing wrong with always exposing it. The bridge itself isn't going to be using the backlight anyway. Rather the panel itself should be using the backlight device exposed by PS8622 or some separate backlight device. To illustrate by an example: ps8622: ... { compatible = parade,ps8622; ... }; panel { ... backlight = ps8622; ... }; No, if ps8622 backlight control is used, we need not specify the backlight phandle for the panel driver. Somehow, ps8622 internal circuitry keeps the bootup glitch free :) Backlight control and panel controls can be separate then. But they shouldn't. Your panel driver should always be the one to control backlight. How else is the bridge supposed to know when to turn backlight on or off? What you did in v6 of this series was look up a backlight device and then not use it. That seemed unnecessary. Looking at v6 again the reason for getting a phandle to the backlight was so that the device itself did not expose its own backlight controlling circuitry if an external one was being used. But since the bridge has no business controlling the backlight, having the backlight phandle in the bridge is not correct. So I think what you could do in the driver instead is always expose the backlight device. If the panel used a different backlight, the PS8622's internal on simply wouldn't be accessed. It would still be possible to control the backlight in sysfs, but that shouldn't be a problem (only root can access it) That would be like simple exposing a feature which cannot be used by the user, ideally which should not be used by the user. And it won't be used unless they
Re: [PATCH V6 6/8] drm/bridge: Modify drm_bridge core to support driver model
Hi Thierry, On Monday 22 September 2014 09:40:38 Thierry Reding wrote: On Wed, Sep 17, 2014 at 12:27:13PM +0300, Laurent Pinchart wrote: On Wednesday 17 September 2014 14:37:30 Ajay kumar wrote: On Mon, Sep 15, 2014 at 11:07 PM, Laurent Pinchart wrote: Hi Ajay, Thank you for the patch. I think we're moving in the right direction, but we're not there yet. On Saturday 26 July 2014 00:52:08 Ajay Kumar wrote: This patch tries to seperate drm_bridge implementation into 2 parts, a drm part and a non_drm part. A set of helper functions are defined in this patch to make bridge driver probe independent of the drm flow. The bridge devices register themselves on a lookup table when they get probed by calling drm_bridge_add_for_lookup. The parent encoder driver waits till the bridge is available in the lookup table(by calling of_drm_find_bridge) and then continues with its initialization. Before the introduction of the component framework I would have said this is the way to go. Now, I think bridges should register themselves as components, and the DRM master driver should use the component framework to get a reference to the bridges it needs. Well, I have modified the bridge framework exactly the way Thierry wanted it to be, I mean the same way the current panel framework is. And, I don't think there is a problem with that. What problem are you facing with current bridge implementation? What is the advantage of using the component framework to register bridges? There are several advantages. - The component framework has been designed with this exact problem in mind, piecing multiple components into a display device. No. Component framework was designed with multi-device drivers in mind. That is, drivers that need to combine two or more platform devices into a single logical device. Typically that includes display controllers and encoders (in various looks) for DRM. I disagree. AFAIK the component framework was designed to easily combine multiple devices into a single logical device, regardless of which bus each device is connected to. That's what makes the component framework useful : it allows master drivers to build logical devices from heterogeneous components without having to use one API per bus and/or component type. If the only goal had been to combine platform devices on an SoC, simpler device-specific solutions would likely have been used instead. Panels and bridges are in my opinion different because they are outside of the DRM driver. They aren't part of the device complex that an SoC provides. They represent hardware that is external to the SoC and the DRM driver and can be shared across SoCs. They represent hardware external to the SoC, but internal to the logical DRM device. Forcing panels and bridges to register as components will require all drivers to implement master/component support solely for accessing this external hardware. What you're suggesting is like saying that clocks or regulators should register as components so that their users can get them that way. In fact by that argument everything that's referenced by phandle would need to register as component (PHYs, LEDs, GPIOs, I2C controllers, ...). No, that's very different. The device you list are clearly external resources, while the bridges and panels are components part of a logical display device. This patch set introduces yet another framework, without any compelling reason as far as I can see. Today DRM drivers already need to use three different frameworks (component, I2C slave encoder and panel), and we're adding a fourth oneto make the mess even messier. Panel and bridge aren't really frameworks. Rather they are a simple registry to allow drivers to register panels and bridges and display drivers to look them up. Regardless of how you call them, we have three interfaces. This is really a headlong rush, we need to stop and fix the design mistakes. Can you point out specific design mistakes? I don't see any, but I'm obviously biased. The slave encoder / bridge split is what I consider a design mistake. Those two interfaces serve the same purpose, they should really be merged. - The component framework solves the probe ordering problem. Bridges can use deferred probing, but when a bridge requires a resources (such as a clock for instance) provided by the display controller, this will break. Panel and bridges can support deferred probing without the component framework just fine. Not if the bridge requires a clock provided by the display controller, in which case there's a dependency loop. Without this patchset, you cannot bring an X server based display on snow and peach_pit. Also, day by day the number of platforms using drm_bridge is increasing. That's exactly why I'd like to use the component framework now,
Re: [PATCH] ASoC: samsung: Remove PCM support for WM8580 on SMDK
On Thu, Sep 18, 2014 at 12:42:16PM +0200, Paul Bolle wrote: On Thu, 2014-09-04 at 18:02 +0200, Arnd Bergmann wrote: I think it would be nice if you could submit a patch to remove the drivers from ASoC, then we can see if anybody complains. Also the same thing for v3.17-rc5 and next-20140918. So let's see if we can remove this driver too. Done on top of next-20140918. Again untested. 8 From: Paul Bolle pebo...@tiscali.nl Again, please follow the patch submission process in SubmittingPatches. signature.asc Description: Digital signature
Re: [PATCH] ASoC: samsung: Remove goni or aquila with the WM8994
On Thu, Sep 18, 2014 at 11:57:07PM +0200, Paul Bolle wrote: Mark Brown schreef op do 18-09-2014 om 10:57 [-0700]: On Thu, Sep 18, 2014 at 11:43:29AM +0200, Paul Bolle wrote: Done on top of next-20140918. Untested. -8- From: Paul Bolle pebo...@tiscali.nl Please follow the patch submission process in SubmittingPatches. Could you please be more specific? What part of SubmittingPatches have I skipped? Is it the perhaps the use of scissors? Or the superfluous From: line? git am -c appears to handle this message (and the very similar message I also sent today) just fine. It's the fact that you've included the patch in the middle of the reply to another mail in the middle of a thread after some other stuff; you can tell this isn't the normal process by observing that your mail doesn't look visually close to a normal patch submission - if that's the case then people aren't likely to sit around and try to figure out some magic set of git options needed, manually edit or otherwise special case to special case it unless it's *really* important. signature.asc Description: Digital signature
Re: [RFC PATCH 2/5] devfreq: event: Add exynos-ppmu devfreq evnet driver
Hi Paul, On 09/12/2014 05:41 PM, Paul Bolle wrote: On Fri, 2014-09-05 at 13:30 +0900, Chanwoo Choi wrote: This patch add exynos-ppmu devfreq event driver to provider raw data about the utilization of each IP in Exynos SoC series. Signed-off-by: Chanwoo Choi cw00.c...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/devfreq/Kconfig | 10 + drivers/devfreq/event/Makefile | 1 + drivers/devfreq/event/exynos-ppmu.c | 410 3 files changed, 421 insertions(+) create mode 100644 drivers/devfreq/event/exynos-ppmu.c diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig index ef839e7..4fbbcea 100644 --- a/drivers/devfreq/Kconfig +++ b/drivers/devfreq/Kconfig @@ -90,4 +90,14 @@ config ARM_EXYNOS5_BUS_DEVFREQ comment DEVFREQ Event Drivers +config DEVFREQ_EVENT_EXYNOS_PPMU +bool EXYNOS PPMU (Performance Profiling Monitoring Unit) DEVFREQ event Driver +depends on ARCH_EXYNOS +select ARCH_HAS_OPP This select statement can be dropped: see commit 78c5e0bb145d (PM / OPP: Remove ARCH_HAS_OPP). I'll drop it. By the way: there's a typo in the commit summary (evnet). My mistake, I'll fix typo. Thanks for your review. Chanwoo Choi +select PM_OPP +help + This add the DEVFREQ event driver for Exynos SoC. It provides PPMU + (Performance Profiling Monitoring Unit) counters to estimate the + utilization of each module. + endif # PM_DEVFREQ Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6 6/8] drm/bridge: Modify drm_bridge core to support driver model
On Tue, Sep 23, 2014 at 03:29:13AM +0300, Laurent Pinchart wrote: Hi Thierry, On Monday 22 September 2014 09:40:38 Thierry Reding wrote: On Wed, Sep 17, 2014 at 12:27:13PM +0300, Laurent Pinchart wrote: On Wednesday 17 September 2014 14:37:30 Ajay kumar wrote: On Mon, Sep 15, 2014 at 11:07 PM, Laurent Pinchart wrote: Hi Ajay, Thank you for the patch. I think we're moving in the right direction, but we're not there yet. On Saturday 26 July 2014 00:52:08 Ajay Kumar wrote: This patch tries to seperate drm_bridge implementation into 2 parts, a drm part and a non_drm part. A set of helper functions are defined in this patch to make bridge driver probe independent of the drm flow. The bridge devices register themselves on a lookup table when they get probed by calling drm_bridge_add_for_lookup. The parent encoder driver waits till the bridge is available in the lookup table(by calling of_drm_find_bridge) and then continues with its initialization. Before the introduction of the component framework I would have said this is the way to go. Now, I think bridges should register themselves as components, and the DRM master driver should use the component framework to get a reference to the bridges it needs. Well, I have modified the bridge framework exactly the way Thierry wanted it to be, I mean the same way the current panel framework is. And, I don't think there is a problem with that. What problem are you facing with current bridge implementation? What is the advantage of using the component framework to register bridges? There are several advantages. - The component framework has been designed with this exact problem in mind, piecing multiple components into a display device. No. Component framework was designed with multi-device drivers in mind. That is, drivers that need to combine two or more platform devices into a single logical device. Typically that includes display controllers and encoders (in various looks) for DRM. I disagree. AFAIK the component framework was designed to easily combine multiple devices into a single logical device, regardless of which bus each device is connected to. That's what makes the component framework useful : it allows master drivers to build logical devices from heterogeneous components without having to use one API per bus and/or component type. But this doesn't work really well once you leave the SoC. For component/ master to work you need to usually (i.e. using DT) have access to a device tree node for each of the devices so that you can create a list of needed devices. Once you leave the SoC, the number of combinations that you can have becomes non-deterministic. A driver that wants to pull this off would need to more or less manually look up phandles and traverse from SoC via bridges to panels to find all the devices that make up the logical device. If the only goal had been to combine platform devices on an SoC, simpler device-specific solutions would likely have been used instead. I think one of the goals was to replace any of the simpler device- specific solutions with a generic one. But one of the issues with component/master is that it's viral. That is it requires users to register as master themselves in order to pull in the components. While that makes sense for on-SoC devices I think it's a mistake to use it for external hardware like bridges and panels that can be shared across SoCs. If we make component/master mandatory for bridges and panels, then we also force every driver that wants to use them to implement component/master support. Furthermore I did try a while back to convert the Tegra DRM driver to use component/master and couldn't make it work. When I proposed patches to enhance the API so that it would work for Tegra I got silence on one side and just keep using what you currently have on the other side. So in other words since I can't use component/master on Tegra it means that whatever driver gets added that registers a component can't be used on Tegra either. Panels and bridges are in my opinion different because they are outside of the DRM driver. They aren't part of the device complex that an SoC provides. They represent hardware that is external to the SoC and the DRM driver and can be shared across SoCs. They represent hardware external to the SoC, but internal to the logical DRM device. That depends largely on how you look at it. From my point of view the logical DRM device stops at the connector (well I think it actually stops somewhat earlier already, with connector only being the handle through which the outputs are configured). Panels are, at least theoretically, hotpluggable. That is, if you have a device that has both a panel and HDMI but you never use
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Mon, Sep 22, 2014 at 05:42:41PM +0300, Tomi Valkeinen wrote: On 22/09/14 11:26, Thierry Reding wrote: On Fri, Sep 19, 2014 at 05:28:37PM +0300, Tomi Valkeinen wrote: On 19/09/14 16:59, Ajay kumar wrote: I am not really able to understand, what's stopping us from using this bridge on a board with complex display connections. To use ps8622 driver, one needs to attach it to the DRM framework. For this, the DRM driver Remember that when we talk about DT bindings, there's no such thing as DRM. We talk about hardware. The same bindings need to work on any operating system. would need the DT node for ps8622 bridge. For which I use a phandle. A complex one could be for example a case where you have two different panels connected to ps8622, and you can switch between the two panels with, say, a gpio. How do you present that with a simple phandle? How do you represent that with a graph? Whether you describe it using a graph or a simple phandle you'll need additional nodes somewhere in between. Perhaps something like this: mux: ... { compatible = gpio-mux-bridge; gpio = gpio 42 GPIO_ACTIVE_HIGH; panel@0 { panel = panel0; }; panel@1 { panel = panel1; }; }; ps8622: ... { bridge = mux; }; panel0: ... { ... }; panel1: ... { ... }; Yes, it's true we need a mux there. But we still have the complication that for panel0 we may need different ps8622 settings than for panel1. Yes, and that's why the bridge should be querying the panel for the information it needs to determine the settings. If that's the case, then I think we'd need to have two output endpoints in ps8622, both going to the mux, and each having the settings for the respective panel. But we'd be lying in DT. It no longer describes the hardware properly. The device only has a single input and a single output with no means to mux anything. Hence the device tree shouldn't be faking multiple inputs or outputs. If some XYZ platform wishes to pick the DT node via a different method, they are always welcome to do it. Just because I am not specifying a video port/endpoint in the DT binding example, would it mean that platform cannot make use of ports in future? If that is the case, I can add something All the platforms share the same bindings for ps8622. If you now specify that ps8622 bindings use a simple phandle, then anyone who uses ps8622 should support that. Of course the bindings can be extended in the future. In that case the drivers need to support both the old and the new bindings, which is always a hassle. Generally speaking, I sense that we have different views of how display devices and drivers are structured. You say If some XYZ platform wishes to pick the DT node via a different method, they are always welcome to do it.. This sounds to me that you see the connections between display devices as something handled by a platform specific driver. I, on the other hand, see connections between display devices as common properties. Say, we could have a display board, with a panel and an encoder and maybe some other components, which takes parallel RGB as input. The same display board could as well be connected to an OMAP board or to an Exynos board. I think the exact same display-board.dtsi file, which describes the devices and connections in the display board, should be usable on both OMAP and Exynos platforms. This means we need to have a common way to describe video devices, just as we have for other things. A common way to describe devices in DT isn't going to give you that. The device tree is completely missing any information about how to access an extension board like that. The operating system defines the API by which the board can be accessed. I imagine that this would work by making the first component of the board a bridge of some sort and then every driver that supports that type of bridge (ideally just a generic drm_bridge) would also work with that display board. I'm not sure I follow. Obviously there needs to be board specific .dts file that brings the board and the display board together. So, say, the display-board.dtsi has a i2c touchscreen node, but the board.dts will tell which i2c bus it's connected to. Well, now as I wrote that, I wonder if that's possible... A node needs to have a parent, and for i2c that must be the i2c master. Is that something the DT overlays/fragments or such are supposed to handle? But let's only think about the video path for now. We could have an encoder and a panel on the board. We could describe the video path between the encoder and the panel in the display-board.dts as that is fixed. Then all that's needed in the board.dts
Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
On Tue, Sep 23, 2014 at 03:00:37AM +0300, Laurent Pinchart wrote: On Monday 22 September 2014 13:35:15 Thierry Reding wrote: On Mon, Sep 22, 2014 at 04:53:22PM +0530, Ajay kumar wrote: On Mon, Sep 22, 2014 at 4:11 PM, Thierry Reding wrote: On Mon, Sep 22, 2014 at 02:01:38PM +0530, Ajay kumar wrote: On Mon, Sep 22, 2014 at 1:40 PM, Thierry Reding wrote: On Thu, Sep 18, 2014 at 11:20:40AM +0530, Ajay kumar wrote: On Wed, Sep 17, 2014 at 9:52 PM, Tomi Valkeinen wrote: On 17/09/14 17:29, Ajay kumar wrote: On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen wrote: On 27/08/14 17:39, Ajay Kumar wrote: Add documentation for DT properties supported by ps8622/ps8625 eDP-LVDS converter. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- .../devicetree/bindings/video/bridge/ps8622.txt| 20 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt new file mode 100644 index 000..0ec8172 --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt @@ -0,0 +1,20 @@ +ps8622-bridge bindings + +Required properties: + - compatible: parade,ps8622 or parade,ps8625 + - reg: first i2c address of the bridge + - sleep-gpios: OF device-tree gpio specification for PD_ pin. + - reset-gpios: OF device-tree gpio specification for RST_ pin. + +Optional properties: + - lane-count: number of DP lanes to use + - use-external-pwm: backlight will be controlled by an external PWM What does this mean? That the backlight support from ps8625 is not used? If so, maybe disable-pwm or something? use-external-pwm or disable-bridge-pwm would be better. Well, the properties are about the bridge. use-external-pwm means that the bridge uses an external PWM, which, if I understood correctly, is not what the property is about. disable-bridge-pwm is ok, but the bridge there is extra. The properties are about the bridge, so it's implicit. Ok. I will use disable-pwm. Why is this even necessary? According to the datasheet this device has circuitry for backlight control. If so, I'd expect it to expose either a backlight device or a PWM device. That way unless somebody is using the backlight/PWM exposed by the bridge the bridge can simply disable PWM. The driver does expose a backlight device. And, the decision(whether to expose a backlight device or not) is made based on the DT flag use-external-pwm. This was discussed before, and you suggested to use the boolean property, refer to this link: http://lists.freedesktop.org/archives/dri-devel/2014-July/065048.html I think you misunderstood what I said, or maybe I didn't explain clearly what I meant. If the PS8622 provides a backlight there's nothing wrong with always exposing it. The bridge itself isn't going to be using the backlight anyway. Rather the panel itself should be using the backlight device exposed by PS8622 or some separate backlight device. To illustrate by an example: ps8622: ... { compatible = parade,ps8622; ... }; panel { ... backlight = ps8622; ... }; No, if ps8622 backlight control is used, we need not specify the backlight phandle for the panel driver. Somehow, ps8622 internal circuitry keeps the bootup glitch free :) Backlight control and panel controls can be separate then. But they shouldn't. Your panel driver should always be the one to control backlight. How else is the bridge supposed to know when to turn backlight on or off? What you did in v6 of this series was look up a backlight device and then not use it. That seemed unnecessary. Looking at v6 again the reason for getting a phandle to the backlight was so that the device itself did not expose its own backlight controlling circuitry if an external one was being used. But since the bridge has no business controlling the backlight, having the backlight phandle in the bridge is not correct. So I think what you could do in the driver instead is always expose the backlight device. If the panel used a different backlight, the PS8622's internal on simply wouldn't be accessed. It would still be possible to control the backlight in sysfs, but that shouldn't be a problem (only root can