| Message ID | 1360912200-26377-6-git-send-email-vikas.sajjan@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 02/15/2013 08:10 AM, Vikas Sajjan wrote: > Adds FIMD DT binding documentation both SoC and Board, with an example > > Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> > --- > .../devicetree/bindings/drm/exynos/fimd.txt | 37 ++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/drm/exynos/fimd.txt > > diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt b/Documentation/devicetree/bindings/drm/exynos/fimd.txt > new file mode 100644 > index 0000000..bec9d07 > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/exynos/fimd.txt > @@ -0,0 +1,37 @@ > +Device-Tree bindings for fimd driver > + > +FIMD stands for Fully Interactive Mobile Display, is the Display Controller for > +the Exynos series of SoCs which transfers the image data from a video buffer > +located in the system memory to an external LCD interface. > + > +Required properties: > +- compatible := value should be "samsung,exynos5-fimd" or "samsung,exynos4-fimd" What about older SoCs like S5Pv210 ? There is the FIMD IP block in those SoCs as well. There are also differences in the FIMD IP block across various SoC version, so either you need to list the quirks in the bindings or use an appropriate compatible properties if there are significant differences across FIMDs that make them not really compatible. > +- reg := physical base address of the fimd and length of memory mapped region > +- interrupt-parent := reference to the interrupt combiner node with phandle > +- interrupts := interrupt number from the combiner to the cpu These are actually 3 interrupts. Can you please document what they are and in what order should be listed in the interrupts property ? > +- pinctrl := property defining the pinctrl configurations with a phandle > +- pinctrl-names := name of the pinctrl > + > +Optional Properties: > +- samsung,power-domain := power domain property defined with a phandle > +- status := property defining the status of the node I don't think this standard property needs to be documented here. > + > +Example: > + > +SoC specific DT Entry: > + > + fimd@11c00000 { > + compatible = "samsung,exynos4-fimd"; > + interrupt-parent = <&combiner>; > + reg = <0x11c00000 0x20000>; > + interrupts = <11 1>, <11 0>, <11 2>; Why this order exactly ? Because currently the driver expects VSYNC interrupt specifier in the first entry and it won't work when you put any other interrupt specifier there ? The interrupts order really needs to be specified above, otherwise you need to look at the bindings implementation in the driver to figure out why it didn't work when you put the interrupt in different order... see confused and angry faces of developers ? ... :) If I'm not mistaken we have "Video Frame" (VSYNC), "I80 Interface" and "FIFO Level" interrupts here. > + }; Thanks,
Hi Sylwester, thannks for the review. On 15 February 2013 16:08, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > Hi, > > On 02/15/2013 08:10 AM, Vikas Sajjan wrote: >> Adds FIMD DT binding documentation both SoC and Board, with an example >> >> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> >> --- >> .../devicetree/bindings/drm/exynos/fimd.txt | 37 ++++++++++++++++++++ >> 1 file changed, 37 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/drm/exynos/fimd.txt >> >> diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt b/Documentation/devicetree/bindings/drm/exynos/fimd.txt >> new file mode 100644 >> index 0000000..bec9d07 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/exynos/fimd.txt >> @@ -0,0 +1,37 @@ >> +Device-Tree bindings for fimd driver >> + >> +FIMD stands for Fully Interactive Mobile Display, is the Display Controller for >> +the Exynos series of SoCs which transfers the image data from a video buffer >> +located in the system memory to an external LCD interface. >> + >> +Required properties: >> +- compatible := value should be "samsung,exynos5-fimd" or "samsung,exynos4-fimd" > > What about older SoCs like S5Pv210 ? There is the FIMD IP block in those SoCs > as well. There are also differences in the FIMD IP block across various SoC > version, so either you need to list the quirks in the bindings or use an > appropriate compatible properties if there are significant differences across > FIMDs that make them not really compatible. > as of now, I was working on Exynos4 and Exynos5 SoC. have to really see the differences in the previous SoC and how to handle all those FIMD IPs in the driver. if you know the differences between these FIMD IPs, please let me know. >> +- reg := physical base address of the fimd and length of memory mapped region >> +- interrupt-parent := reference to the interrupt combiner node with phandle >> +- interrupts := interrupt number from the combiner to the cpu > > These are actually 3 interrupts. Can you please document what they are > and in what order should be listed in the interrupts property ? > Sure, will mention the all the 3 interrupts and the order. >> +- pinctrl := property defining the pinctrl configurations with a phandle >> +- pinctrl-names := name of the pinctrl >> + >> +Optional Properties: >> +- samsung,power-domain := power domain property defined with a phandle >> +- status := property defining the status of the node > > I don't think this standard property needs to be documented here. > fine. will remove. >> + >> +Example: >> + >> +SoC specific DT Entry: >> + >> + fimd@11c00000 { >> + compatible = "samsung,exynos4-fimd"; >> + interrupt-parent = <&combiner>; >> + reg = <0x11c00000 0x20000>; >> + interrupts = <11 1>, <11 0>, <11 2>; > > Why this order exactly ? Because currently the driver expects VSYNC > interrupt specifier in the first entry and it won't work when you put > any other interrupt specifier there ? > > The interrupts order really needs to be specified above, otherwise > you need to look at the bindings implementation in the driver to > figure out why it didn't work when you put the interrupt in different > order... see confused and angry faces of developers ? ... :) > we have 3 interrupts and the Interrupt combiner order is FIFO Level , VSYNC and LCD_SYSTEM. since we only use VSYNC interrupt , In the driver to get the interrupt number res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { dev_err(dev, "irq request failed.\n"); return -ENXIO; } ctx->irq = res->start; I am passing the 3rd parameter as '0' , to get the VSYNC interrupt, and hence in the DTSI file i have it in the order VSYNC, FIFO LEVEL and LCD_SYSTEM. what I can do is the pass the 3rd parameter as '1' , and rectify the order in DTSI file as FIFO LEVEL, VSYNC and LCD_SYSTEM. > If I'm not mistaken we have "Video Frame" (VSYNC), "I80 Interface" > and "FIFO Level" interrupts here. > >> + }; > > > Thanks, > -- > Sylwester Nawrocki > Samsung Poland R&D Center
Hi, On 02/18/2013 11:51 AM, Vikas Sajjan wrote: > On 15 February 2013 16:08, Sylwester Nawrocki<s.nawrocki@samsung.com> wrote: >> On 02/15/2013 08:10 AM, Vikas Sajjan wrote: >>> Adds FIMD DT binding documentation both SoC and Board, with an example >>> >>> Signed-off-by: Vikas Sajjan<vikas.sajjan@linaro.org> >>> --- >>> .../devicetree/bindings/drm/exynos/fimd.txt | 37 ++++++++++++++++++++ >>> 1 file changed, 37 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/drm/exynos/fimd.txt >>> >>> diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt b/Documentation/devicetree/bindings/drm/exynos/fimd.txt >>> new file mode 100644 >>> index 0000000..bec9d07 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/drm/exynos/fimd.txt Wouldn't Documentation/devicetree/bindings/video/exynos-fimd.txt be more appropriate location for this file ? There is also framebuffer driver for FIMD. Even if the framebuffer driver gets phased out it still feels like a more intuitive location for a display controller bindings. This is just my humble suggestion though. I think you should CC the device tree mailing list and the maintainers. >>> @@ -0,0 +1,37 @@ >>> +Device-Tree bindings for fimd driver >>> + >>> +FIMD stands for Fully Interactive Mobile Display, is the Display Controller for >>> +the Exynos series of SoCs which transfers the image data from a video buffer >>> +located in the system memory to an external LCD interface. >>> + >>> +Required properties: >>> +- compatible := value should be "samsung,exynos5-fimd" or "samsung,exynos4-fimd" >> >> What about older SoCs like S5Pv210 ? There is the FIMD IP block in those SoCs >> as well. There are also differences in the FIMD IP block across various SoC >> version, so either you need to list the quirks in the bindings or use an >> appropriate compatible properties if there are significant differences across >> FIMDs that make them not really compatible. >> > as of now, I was working on Exynos4 and Exynos5 SoC. have to really > see the differences in the previous SoC and how to handle all those > FIMD IPs in the driver. > if you know the differences between these FIMD IPs, please let me know. Please have a look at drivers/video/s3c-fb.c and all driver_data structures defined for various SoCs. [...] > we have 3 interrupts and the Interrupt combiner order is FIFO Level , > VSYNC and LCD_SYSTEM. > since we only use VSYNC interrupt , In the driver to get the interrupt number > > res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > if (!res) { > dev_err(dev, "irq request failed.\n"); > return -ENXIO; > } > > ctx->irq = res->start; > > I am passing the 3rd parameter as '0' , to get the VSYNC interrupt, > and hence in the DTSI file i have it in the order VSYNC, FIFO > LEVEL and LCD_SYSTEM. Yes, I'm aware what's going on here. Since I had to change the interrupts order when I used one of the first version of patches adding device tree support for FIMD, when I needed working display to test the camera. > what I can do is the pass the 3rd parameter as '1' , and rectify the > order in DTSI file as FIFO LEVEL, VSYNC and LCD_SYSTEM. Then you would have to do it only for dt case. I guess it is okay to require VSYNC as the first interrupt. I'm fine with that, as long as it is properly documented. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 19 February 2013 03:16, Sylwester Nawrocki <sylvester.nawrocki@gmail.com> wrote: > Hi, > > > On 02/18/2013 11:51 AM, Vikas Sajjan wrote: >> >> On 15 February 2013 16:08, Sylwester Nawrocki<s.nawrocki@samsung.com> >> wrote: >>> >>> On 02/15/2013 08:10 AM, Vikas Sajjan wrote: >>> >>>> Adds FIMD DT binding documentation both SoC and Board, with an example >>>> >>>> Signed-off-by: Vikas Sajjan<vikas.sajjan@linaro.org> >>>> --- >>>> .../devicetree/bindings/drm/exynos/fimd.txt | 37 >>>> ++++++++++++++++++++ >>>> 1 file changed, 37 insertions(+) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/drm/exynos/fimd.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt >>>> b/Documentation/devicetree/bindings/drm/exynos/fimd.txt >>>> new file mode 100644 >>>> index 0000000..bec9d07 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/drm/exynos/fimd.txt > > > Wouldn't Documentation/devicetree/bindings/video/exynos-fimd.txt be more > appropriate location for this file ? There is also framebuffer driver for > FIMD. Even if the framebuffer driver gets phased out it still feels like > a more intuitive location for a display controller bindings. > This is just my humble suggestion though. > I think we can move it there. > I think you should CC the device tree mailing list and the maintainers. > > will do. >>>> @@ -0,0 +1,37 @@ >>>> +Device-Tree bindings for fimd driver >>>> + >>>> +FIMD stands for Fully Interactive Mobile Display, is the Display >>>> Controller for >>>> +the Exynos series of SoCs which transfers the image data from a video >>>> buffer >>>> +located in the system memory to an external LCD interface. >>>> + >>>> +Required properties: >>>> +- compatible := value should be "samsung,exynos5-fimd" or >>>> "samsung,exynos4-fimd" >>> >>> >>> What about older SoCs like S5Pv210 ? There is the FIMD IP block in those >>> SoCs >>> as well. There are also differences in the FIMD IP block across various >>> SoC >>> version, so either you need to list the quirks in the bindings or use an >>> appropriate compatible properties if there are significant differences >>> across >>> FIMDs that make them not really compatible. >>> >> as of now, I was working on Exynos4 and Exynos5 SoC. have to really >> see the differences in the previous SoC and how to handle all those >> FIMD IPs in the driver. >> if you know the differences between these FIMD IPs, please let me know. > > > Please have a look at drivers/video/s3c-fb.c and all driver_data structures > defined for various SoCs. > > [...] > I went through the driver_data structures define for s5p64x0,s3c2443, s5pv210, s5pc100 ad 64xx SoCs. there are some differences w.r.t number of windows, osd_stride length and regsiter offsets. but I dont have access to all the above mentioned Boards and so that I can modify and test on each one of them. >> we have 3 interrupts and the Interrupt combiner order is FIFO Level , >> VSYNC and LCD_SYSTEM. >> since we only use VSYNC interrupt , In the driver to get the interrupt >> number >> >> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> if (!res) { >> dev_err(dev, "irq request failed.\n"); >> return -ENXIO; >> } >> >> ctx->irq = res->start; >> >> I am passing the 3rd parameter as '0' , to get the VSYNC interrupt, >> and hence in the DTSI file i have it in the order VSYNC, FIFO >> LEVEL and LCD_SYSTEM. > > > Yes, I'm aware what's going on here. Since I had to change the interrupts > order when I used one of the first version of patches adding device tree > support for FIMD, when I needed working display to test the camera. > > >> what I can do is the pass the 3rd parameter as '1' , and rectify the >> order in DTSI file as FIFO LEVEL, VSYNC and LCD_SYSTEM. > > > Then you would have to do it only for dt case. I guess it is okay to > require VSYNC as the first interrupt. I'm fine with that, as long as > it is properly documented. > So, I shall keep the logic as is, and just document it well. is it ok.? > -- > > Regards, > Sylwester
On 21 February 2013 12:34, Vikas Sajjan <vikas.sajjan@linaro.org> wrote: > Hi, > > On 19 February 2013 03:16, Sylwester Nawrocki > <sylvester.nawrocki@gmail.com> wrote: >> Hi, >> >> >> On 02/18/2013 11:51 AM, Vikas Sajjan wrote: >>> >>> On 15 February 2013 16:08, Sylwester Nawrocki<s.nawrocki@samsung.com> >>> wrote: >>>> >>>> On 02/15/2013 08:10 AM, Vikas Sajjan wrote: >>>> >>>>> Adds FIMD DT binding documentation both SoC and Board, with an example >>>>> >>>>> Signed-off-by: Vikas Sajjan<vikas.sajjan@linaro.org> >>>>> --- >>>>> .../devicetree/bindings/drm/exynos/fimd.txt | 37 >>>>> ++++++++++++++++++++ >>>>> 1 file changed, 37 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/drm/exynos/fimd.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt >>>>> b/Documentation/devicetree/bindings/drm/exynos/fimd.txt >>>>> new file mode 100644 >>>>> index 0000000..bec9d07 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/drm/exynos/fimd.txt >> >> >> Wouldn't Documentation/devicetree/bindings/video/exynos-fimd.txt be more >> appropriate location for this file ? There is also framebuffer driver for >> FIMD. Even if the framebuffer driver gets phased out it still feels like >> a more intuitive location for a display controller bindings. >> This is just my humble suggestion though. >> > I think we can move it there. > >> I think you should CC the device tree mailing list and the maintainers. >> >> > will do. > >>>>> @@ -0,0 +1,37 @@ >>>>> +Device-Tree bindings for fimd driver >>>>> + >>>>> +FIMD stands for Fully Interactive Mobile Display, is the Display >>>>> Controller for >>>>> +the Exynos series of SoCs which transfers the image data from a video >>>>> buffer >>>>> +located in the system memory to an external LCD interface. >>>>> + >>>>> +Required properties: >>>>> +- compatible := value should be "samsung,exynos5-fimd" or >>>>> "samsung,exynos4-fimd" >>>> >>>> >>>> What about older SoCs like S5Pv210 ? There is the FIMD IP block in those >>>> SoCs >>>> as well. There are also differences in the FIMD IP block across various >>>> SoC >>>> version, so either you need to list the quirks in the bindings or use an >>>> appropriate compatible properties if there are significant differences >>>> across >>>> FIMDs that make them not really compatible. >>>> >>> as of now, I was working on Exynos4 and Exynos5 SoC. have to really >>> see the differences in the previous SoC and how to handle all those >>> FIMD IPs in the driver. >>> if you know the differences between these FIMD IPs, please let me know. >> >> >> Please have a look at drivers/video/s3c-fb.c and all driver_data structures >> defined for various SoCs. >> >> [...] >> > I went through the driver_data structures define for s5p64x0,s3c2443, > s5pv210, s5pc100 ad 64xx SoCs. > there are some differences w.r.t number of windows, osd_stride length > and regsiter offsets. > but I dont have access to all the above mentioned Boards and so that > I can modify and test on each one of them. > as far as i know this file exynos_drm_fimd.c is meant only for exynos4 onwards SoCs, ( I think Mr. Inki Dae can throw more light on this ) and also the previous boards based on s5p64x0,s3c2443,s5pv210, s5pc100 and 64xx, dont have DT support. also in the function drm_fimd_get_driver_data(), there is provision to get the NON-DT "driver data" as well. So somebody who wants to use this driver for the above mentioned SoCs can add the driver-data. so wanted to know how you want me to go about it. >>> we have 3 interrupts and the Interrupt combiner order is FIFO Level , >>> VSYNC and LCD_SYSTEM. >>> since we only use VSYNC interrupt , In the driver to get the interrupt >>> number >>> >>> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >>> if (!res) { >>> dev_err(dev, "irq request failed.\n"); >>> return -ENXIO; >>> } >>> >>> ctx->irq = res->start; >>> >>> I am passing the 3rd parameter as '0' , to get the VSYNC interrupt, >>> and hence in the DTSI file i have it in the order VSYNC, FIFO >>> LEVEL and LCD_SYSTEM. >> >> >> Yes, I'm aware what's going on here. Since I had to change the interrupts >> order when I used one of the first version of patches adding device tree >> support for FIMD, when I needed working display to test the camera. >> >> >>> what I can do is the pass the 3rd parameter as '1' , and rectify the >>> order in DTSI file as FIFO LEVEL, VSYNC and LCD_SYSTEM. >> >> >> Then you would have to do it only for dt case. I guess it is okay to >> require VSYNC as the first interrupt. I'm fine with that, as long as >> it is properly documented. >> > So, I shall keep the logic as is, and just document it well. is it ok.? >> -- >> >> Regards, >> Sylwester > > > > -- > Thanks and Regards > Vikas Sajjan
Hi Vikas, On 02/22/2013 08:06 AM, Vikas Sajjan wrote: >>>>>> +++ b/Documentation/devicetree/bindings/drm/exynos/fimd.txt [...] >>>>>> @@ -0,0 +1,37 @@ >>>>>> +Device-Tree bindings for fimd driver Perhaps something like: Exynos SoC display controller (FIMD) would do ? Even though bindings are to be used by some driver we should be focusing on describing the actual hardware. >>>>>> +FIMD stands for Fully Interactive Mobile Display, is the Display >>>>>> Controller for >>>>>> +the Exynos series of SoCs which transfers the image data from a video >>>>>> buffer >>>>>> +located in the system memory to an external LCD interface. >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible := value should be "samsung,exynos5-fimd" or >>>>>> "samsung,exynos4-fimd" >>>>> >>>>> >>>>> What about older SoCs like S5Pv210 ? There is the FIMD IP block in those >>>>> SoCs >>>>> as well. There are also differences in the FIMD IP block across various >>>>> SoC >>>>> version, so either you need to list the quirks in the bindings or use an >>>>> appropriate compatible properties if there are significant differences >>>>> across >>>>> FIMDs that make them not really compatible. >>>>> >>>> as of now, I was working on Exynos4 and Exynos5 SoC. have to really >>>> see the differences in the previous SoC and how to handle all those >>>> FIMD IPs in the driver. >>>> if you know the differences between these FIMD IPs, please let me know. >>> >>> >>> Please have a look at drivers/video/s3c-fb.c and all driver_data structures >>> defined for various SoCs. >>> >>> [...] >>> >> I went through the driver_data structures define for s5p64x0,s3c2443, >> s5pv210, s5pc100 ad 64xx SoCs. >> there are some differences w.r.t number of windows, osd_stride length >> and regsiter offsets. >> but I dont have access to all the above mentioned Boards and so that >> I can modify and test on each one of them. I don't ask you to add support for all existing FIMD variants. Please just make sure the bindings you're documenting here, with focus on the Exynos SoCs, are also usable for older SoC series. > as far as i know this file exynos_drm_fimd.c is meant only for exynos4 > onwards SoCs, Yes, but the bindings are for FIMD IP block and should possibly be consistent across all SoC series. I think we should bear in mind that eventually all these SoCs get device tree support. Actually I've seen already patches for s3c24xx and s3c64xx SoC series. > ( I think Mr. Inki Dae can throw more light on this ) > and also the previous boards based on s5p64x0,s3c2443,s5pv210, > s5pc100 and 64xx, dont have DT support. We can't tell for sure the SoC series listed above won't get device tree support sooner or later, as this is an open source project. > also in the function drm_fimd_get_driver_data(), there is provision to > get the NON-DT "driver data" as well. So somebody who wants to use > this driver for the above mentioned SoCs can add the driver-data. I'm not concerned about this particular driver, only about the DT bindings that are not supposed to change over time. So some care needs to be taken when designing the bindings. > so wanted to know how you want me to go about it. My concern was that you use "samsung,exynos4-fimd" for FIMD IP found on all Exynos4 SoC, whereas there could be some differences for Exynos4210 and Exynos4212/4412. It looks like both are shipped with FIMD rev. 6.0 though. Anyway, still there are some details that may need to be handled, e.g. maximum VCLK frequency is 80 MHz for Exynos4210 FIMD0 and Exynos4412 FIMD (there is only one there in Exynos4412 as opposed to two in Exynos4210), while for Exynos4210 FIMD1 it is only 50 MHz. When there are differences we really care about (and support for FIMD1 is added) I guess things like this could be handled through optional properties. I thought about something like: compatible = "samsung,exynos4210-fimd"; for Exynos4210 and compatible = "samsung,exynos4212-fimd", "samsung,exynos4210-fimd"; for Exynos4212 and Exynos412, but "samsung,exynos4-fimd" seems good enough, since the FIMD IP block appears nearly identical across Exynos4210...Exynos4412. -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Friday 22 of February 2013 12:36:22 Vikas Sajjan wrote: > On 21 February 2013 12:34, Vikas Sajjan <vikas.sajjan@linaro.org> wrote: > > Hi, > > > > On 19 February 2013 03:16, Sylwester Nawrocki > > > > <sylvester.nawrocki@gmail.com> wrote: > >> Hi, > >> > >> On 02/18/2013 11:51 AM, Vikas Sajjan wrote: > >>> On 15 February 2013 16:08, Sylwester > >>> Nawrocki<s.nawrocki@samsung.com> > >>> > >>> wrote: > >>>> On 02/15/2013 08:10 AM, Vikas Sajjan wrote: > >>>>> Adds FIMD DT binding documentation both SoC and Board, with an > >>>>> example > >>>>> > >>>>> Signed-off-by: Vikas Sajjan<vikas.sajjan@linaro.org> > >>>>> --- > >>>>> > >>>>> .../devicetree/bindings/drm/exynos/fimd.txt | 37 > >>>>> > >>>>> ++++++++++++++++++++ > >>>>> > >>>>> 1 file changed, 37 insertions(+) > >>>>> create mode 100644 > >>>>> > >>>>> Documentation/devicetree/bindings/drm/exynos/fimd.txt > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt > >>>>> b/Documentation/devicetree/bindings/drm/exynos/fimd.txt > >>>>> new file mode 100644 > >>>>> index 0000000..bec9d07 > >>>>> --- /dev/null > >>>>> +++ b/Documentation/devicetree/bindings/drm/exynos/fimd.txt > >> > >> Wouldn't Documentation/devicetree/bindings/video/exynos-fimd.txt be > >> more appropriate location for this file ? There is also framebuffer > >> driver for FIMD. Even if the framebuffer driver gets phased out it > >> still feels like a more intuitive location for a display controller > >> bindings. > >> This is just my humble suggestion though. > > > > I think we can move it there. > > > >> I think you should CC the device tree mailing list and the > >> maintainers. > > > > will do. > > > >>>>> @@ -0,0 +1,37 @@ > >>>>> +Device-Tree bindings for fimd driver > >>>>> + > >>>>> +FIMD stands for Fully Interactive Mobile Display, is the Display > >>>>> Controller for > >>>>> +the Exynos series of SoCs which transfers the image data from a > >>>>> video > >>>>> buffer > >>>>> +located in the system memory to an external LCD interface. > >>>>> + > >>>>> +Required properties: > >>>>> +- compatible := value should be "samsung,exynos5-fimd" or > >>>>> "samsung,exynos4-fimd" > >>>> > >>>> What about older SoCs like S5Pv210 ? There is the FIMD IP block in > >>>> those SoCs > >>>> as well. There are also differences in the FIMD IP block across > >>>> various > >>>> SoC > >>>> version, so either you need to list the quirks in the bindings or > >>>> use an appropriate compatible properties if there are significant > >>>> differences across > >>>> FIMDs that make them not really compatible. > >>> > >>> as of now, I was working on Exynos4 and Exynos5 SoC. have to really > >>> see the differences in the previous SoC and how to handle all those > >>> FIMD IPs in the driver. > >>> if you know the differences between these FIMD IPs, please let me > >>> know. > >> > >> Please have a look at drivers/video/s3c-fb.c and all driver_data > >> structures defined for various SoCs. > >> > >> [...] > > > > I went through the driver_data structures define for s5p64x0,s3c2443, > > s5pv210, s5pc100 ad 64xx SoCs. > > there are some differences w.r.t number of windows, osd_stride length > > and regsiter offsets. > > but I dont have access to all the above mentioned Boards and so that > > I can modify and test on each one of them. > > as far as i know this file exynos_drm_fimd.c is meant only for exynos4 > onwards SoCs, This is probably true, as even the name suggests that this is a DRM implementation for Exynos SoCs. However DRM is not the only driver which can handle FIMD. There is also the s3c-fb driver. Since it supports the same hardware, it is also affected by Device Tree bindings you are describing in this patch. > ( I think Mr. Inki Dae can throw more light on this ) > and also the previous boards based on s5p64x0,s3c2443,s5pv210, > s5pc100 and 64xx, dont have DT support. This is true only at the moment. S3C24xx (Heiko Stuebner is working on it) and S3C64xx (I'm working on it) are on good way to have DT support soon. Other S5P SoCs will have to be eventually moved to DT too (or dropped, whatever gets decided by maintainers). Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 23 February 2013 03:07, Sylwester Nawrocki <sylvester.nawrocki@gmail.com> wrote: > Hi Vikas, > > On 02/22/2013 08:06 AM, Vikas Sajjan wrote: >>>>>>> >>>>>>> +++ b/Documentation/devicetree/bindings/drm/exynos/fimd.txt > > [...] > >>>>>>> @@ -0,0 +1,37 @@ >>>>>>> +Device-Tree bindings for fimd driver > > > Perhaps something like: > > Exynos SoC display controller (FIMD) > > would do ? Even though bindings are to be used by some driver we should > be focusing on describing the actual hardware. > OK. will change. > >>>>>>> +FIMD stands for Fully Interactive Mobile Display, is the Display >>>>>>> Controller for >>>>>>> +the Exynos series of SoCs which transfers the image data from a >>>>>>> video >>>>>>> buffer >>>>>>> +located in the system memory to an external LCD interface. >>>>>>> + >>>>>>> +Required properties: >>>>>>> +- compatible := value should be "samsung,exynos5-fimd" or >>>>>>> "samsung,exynos4-fimd" >>>>>> >>>>>> >>>>>> >>>>>> What about older SoCs like S5Pv210 ? There is the FIMD IP block in >>>>>> those >>>>>> SoCs >>>>>> as well. There are also differences in the FIMD IP block across >>>>>> various >>>>>> SoC >>>>>> version, so either you need to list the quirks in the bindings or use >>>>>> an >>>>>> appropriate compatible properties if there are significant differences >>>>>> across >>>>>> FIMDs that make them not really compatible. >>>>>> >>>>> as of now, I was working on Exynos4 and Exynos5 SoC. have to really >>>>> see the differences in the previous SoC and how to handle all those >>>>> FIMD IPs in the driver. >>>>> if you know the differences between these FIMD IPs, please let me know. >>>> >>>> >>>> >>>> Please have a look at drivers/video/s3c-fb.c and all driver_data >>>> structures >>>> defined for various SoCs. >>>> >>>> [...] >>>> >>> I went through the driver_data structures define for s5p64x0,s3c2443, >>> s5pv210, s5pc100 ad 64xx SoCs. >>> there are some differences w.r.t number of windows, osd_stride length >>> and regsiter offsets. >>> but I dont have access to all the above mentioned Boards and so that >>> I can modify and test on each one of them. > > > I don't ask you to add support for all existing FIMD variants. Please just > make sure the bindings you're documenting here, with focus on the Exynos > SoCs, are also usable for older SoC series. > > >> as far as i know this file exynos_drm_fimd.c is meant only for exynos4 >> onwards SoCs, > > > Yes, but the bindings are for FIMD IP block and should possibly be > consistent > across all SoC series. I think we should bear in mind that eventually all > these SoCs get device tree support. Actually I've seen already patches for > s3c24xx and s3c64xx SoC series. > > >> ( I think Mr. Inki Dae can throw more light on this ) >> and also the previous boards based on s5p64x0,s3c2443,s5pv210, >> s5pc100 and 64xx, dont have DT support. > > > We can't tell for sure the SoC series listed above won't get device tree > support sooner or later, as this is an open source project. > > >> also in the function drm_fimd_get_driver_data(), there is provision to >> get the NON-DT "driver data" as well. So somebody who wants to use >> this driver for the above mentioned SoCs can add the driver-data. > > > I'm not concerned about this particular driver, only about the DT bindings > that are not supposed to change over time. So some care needs to be taken > when designing the bindings. > > >> so wanted to know how you want me to go about it. > > > My concern was that you use "samsung,exynos4-fimd" for FIMD IP found on all > Exynos4 SoC, whereas there could be some differences for Exynos4210 and > Exynos4212/4412. It looks like both are shipped with FIMD rev. 6.0 though. > Anyway, still there are some details that may need to be handled, e.g. > maximum > VCLK frequency is 80 MHz for Exynos4210 FIMD0 and Exynos4412 FIMD (there is > only one there in Exynos4412 as opposed to two in Exynos4210), while for > Exynos4210 FIMD1 it is only 50 MHz. When there are differences we really > care about (and support for FIMD1 is added) I guess things like this could > be handled through optional properties. > > I thought about something like: > > compatible = "samsung,exynos4210-fimd"; for Exynos4210 and > compatible = "samsung,exynos4212-fimd", "samsung,exynos4210-fimd"; for > Exynos4212 and Exynos412, > > but "samsung,exynos4-fimd" seems good enough, since the FIMD IP block > appears nearly identical across Exynos4210...Exynos4412. > ok thanks. Will modify the documentation as below. compatible = "samsung, exynos4-fimd"; for Exynos4 SoCs compatible = "samsung, exynos5-fimd" ; for Exynos5 SoCs compatible = "samsung, s3c64xx-fimd" ; for S3C64XX SoCs compatible = "samsung, s3c24xx-fimd" ; for S3C24XX SoCs compatible = "samsung, s5p64x0-fimd" ; for S5P64X0 SoCs compatible = "samsung, s5pc100-fimd" ; for S5PC100 SoCs compatible = "samsung, s5pv210-fimd" ; for S5PV210 SoCs > -- > > Thanks, > Sylwester
On 02/26/2013 05:19 AM, Vikas Sajjan wrote: > ok thanks. Will modify the documentation as below. > > compatible = "samsung, exynos4-fimd"; for Exynos4 SoCs > compatible = "samsung, exynos5-fimd" ; for Exynos5 SoCs > compatible = "samsung, s3c64xx-fimd" ; for S3C64XX SoCs > compatible = "samsung, s3c24xx-fimd" ; for S3C24XX SoCs > compatible = "samsung, s5p64x0-fimd" ; for S5P64X0 SoCs There should be no wildcards in the compatible property. I guess we can just add the two first one above for now and leave other out for when someone actually adds support for these SoCs at the drivers. I suppose we could, for example, have something like: compatible = "samsung,s3c2410-fimd"; // for s3c2410 compatible = "samsung,s3c2440-fimd", "samsung,s3c2410-fimd"; // for s3c2440 etc. But this really needs to be checked with the documentation in each case. > compatible = "samsung, s5pc100-fimd" ; for S5PC100 SoCs > compatible = "samsung, s5pv210-fimd" ; for S5PV210 SoCs -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 26 of February 2013 22:57:15 Sylwester Nawrocki wrote: > On 02/26/2013 05:19 AM, Vikas Sajjan wrote: > > ok thanks. Will modify the documentation as below. > > > > compatible = "samsung, exynos4-fimd"; for Exynos4 SoCs > > compatible = "samsung, exynos5-fimd" ; for Exynos5 SoCs > > compatible = "samsung, s3c64xx-fimd" ; for S3C64XX SoCs > > compatible = "samsung, s3c24xx-fimd" ; for S3C24XX SoCs > > compatible = "samsung, s5p64x0-fimd" ; for S5P64X0 SoCs > > There should be no wildcards in the compatible property. I guess we can > just add the two first one above for now and leave other out for when > someone actually adds support for these SoCs at the drivers. > > I suppose we could, for example, have something like: > > compatible = "samsung,s3c2410-fimd"; // for > s3c2410 compatible = "samsung,s3c2440-fimd", "samsung,s3c2410-fimd"; // > for s3c2440 > > etc. But this really needs to be checked with the documentation > in each case. > > > compatible = "samsung, s5pc100-fimd" ; for S5PC100 SoCs > > compatible = "samsung, s5pv210-fimd" ; for S5PV210 SoCs We should stick to the rule that compatible value should be named after first specific SoC model in which this particular IP version was included. So this is what I would suggest: compatible = "samsung,s3c2443-fimd"; // for S3C24XX SoCs compatible = "samsung,s3c6400-fimd"; // for S3C64XX SoCs compatible = "samsung,s5p6440-fimd"; // for S5P64X0 SoCs compatible = "samsung,s5pc100-fimd"; // for S5PC100 SoC compatible = "samsung,s5pv210-fimd"; // for S5PV210 SoC compatible = "samsung,exynos4210-fimd"; // for Exynos4 SoCs compatible = "samsung,exynos5250-fimd"; // for Exynos5 SoCs (All the hardware variants are taken from s3c-fb driver.) Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/26/2013 11:11 PM, Tomasz Figa wrote: > We should stick to the rule that compatible value should be named after > first specific SoC model in which this particular IP version was included. > > So this is what I would suggest: > > compatible = "samsung,s3c2443-fimd"; // for S3C24XX SoCs > compatible = "samsung,s3c6400-fimd"; // for S3C64XX SoCs > compatible = "samsung,s5p6440-fimd"; // for S5P64X0 SoCs > compatible = "samsung,s5pc100-fimd"; // for S5PC100 SoC > compatible = "samsung,s5pv210-fimd"; // for S5PV210 SoC > compatible = "samsung,exynos4210-fimd"; // for Exynos4 SoCs > compatible = "samsung,exynos5250-fimd"; // for Exynos5 SoCs Yes, that looks good to me. I'm just not sure though if there is any reference to "fimd" in the s3c2443 SoC documentation. Or is it simply called lcd controller there ? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt b/Documentation/devicetree/bindings/drm/exynos/fimd.txt new file mode 100644 index 0000000..bec9d07 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/exynos/fimd.txt @@ -0,0 +1,37 @@ +Device-Tree bindings for fimd driver + +FIMD stands for Fully Interactive Mobile Display, is the Display Controller for +the Exynos series of SoCs which transfers the image data from a video buffer +located in the system memory to an external LCD interface. + +Required properties: +- compatible := value should be "samsung,exynos5-fimd" or "samsung,exynos4-fimd" +- reg := physical base address of the fimd and length of memory mapped region +- interrupt-parent := reference to the interrupt combiner node with phandle +- interrupts := interrupt number from the combiner to the cpu +- pinctrl := property defining the pinctrl configurations with a phandle +- pinctrl-names := name of the pinctrl + +Optional Properties: +- samsung,power-domain := power domain property defined with a phandle +- status := property defining the status of the node + +Example: + +SoC specific DT Entry: + + fimd@11c00000 { + compatible = "samsung,exynos4-fimd"; + interrupt-parent = <&combiner>; + reg = <0x11c00000 0x20000>; + interrupts = <11 1>, <11 0>, <11 2>; + }; + +Board specific DT Entry: + + fimd@11c00000 { + samsung,power-domain = <&pd_lcd0>; + pinctrl-0 = <&lcd_sync &lcd_clk &lcd_en &lcd0_data &pwm1_out>; + pinctrl-names = "default"; + status = "okay"; + };
Adds FIMD DT binding documentation both SoC and Board, with an example Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org> --- .../devicetree/bindings/drm/exynos/fimd.txt | 37 ++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/drm/exynos/fimd.txt