| Message ID | 1404241055-3579-1-git-send-email-robdclark@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 07/01/2014 12:57 PM, Rob Clark wrote: > diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt b/Documentation/devicetree/bindings/drm/msm/gpu.txt > new file mode 100644 > index 0000000..6e33efe > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt > @@ -0,0 +1,51 @@ > +Qualcomm adreno/snapdragon GPU > + > +Required properties: > +- compatible: "qcom,adreno-3xx" 3xx won't always be the only game in town. Not that I have a better name, just pointing out that it has a limited shelf life. Jordan
On Wed, Jul 2, 2014 at 10:26 AM, Jordan Crouse <jcrouse@codeaurora.org> wrote: > > On 07/01/2014 12:57 PM, Rob Clark wrote: >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt b/Documentation/devicetree/bindings/drm/msm/gpu.txt >> new file mode 100644 >> index 0000000..6e33efe >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt >> @@ -0,0 +1,51 @@ >> +Qualcomm adreno/snapdragon GPU >> + >> +Required properties: >> +- compatible: "qcom,adreno-3xx" > > > 3xx won't always be the only game in town. Not that I have a better name, > just pointing out that it has a limited shelf life. > yup, I'm expecting we'll add new compatible strings, ie. qcom,adreno-4xx, etc, as we add support for various future generations BR, -R > Jordan > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/02/2014 08:42 AM, Rob Clark wrote: > On Wed, Jul 2, 2014 at 10:26 AM, Jordan Crouse <jcrouse@codeaurora.org> wrote: >> >> On 07/01/2014 12:57 PM, Rob Clark wrote: >>> >>> diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt b/Documentation/devicetree/bindings/drm/msm/gpu.txt >>> new file mode 100644 >>> index 0000000..6e33efe >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt >>> @@ -0,0 +1,51 @@ >>> +Qualcomm adreno/snapdragon GPU >>> + >>> +Required properties: >>> +- compatible: "qcom,adreno-3xx" >> >> >> 3xx won't always be the only game in town. Not that I have a better name, >> just pointing out that it has a limited shelf life. >> > > yup, I'm expecting we'll add new compatible strings, ie. > qcom,adreno-4xx, etc, as we add support for various future generations > > BR, > -R Alright, fair enough. Compatible strings are cheap. Jordan
On Wed, Jul 2, 2014 at 7:26 AM, Jordan Crouse <jcrouse@codeaurora.org> wrote: > On 07/01/2014 12:57 PM, Rob Clark wrote: >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt >> b/Documentation/devicetree/bindings/drm/msm/gpu.txt >> new file mode 100644 >> index 0000000..6e33efe >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt >> @@ -0,0 +1,51 @@ >> +Qualcomm adreno/snapdragon GPU >> + >> +Required properties: >> +- compatible: "qcom,adreno-3xx" > > > 3xx won't always be the only game in town. Not that I have a better name, > just pointing out that it has a limited shelf life. Actually, we prefer to avoid "xx" compatible strings. The compatible should be the first actual device that is part of the xx series, and newer IP should claim to be compatible with their own version plus the original one (if they are possible to use with the same exact programming model and only need extra code to deal with extensions). That way you'll also have a way to tell them apart in the driver if you can't derive it out of register contents, etc. -Olof -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 01, 2014 at 07:57:35PM +0100, Rob Clark wrote: > Now that we (almost) have enough dependencies in place (MMCC, RPM, etc), > add necessary DT support so that we can use drm/msm on upstream kernel. > > Signed-off-by: Rob Clark <robdclark@gmail.com> > --- > Commence bikeshedding :-) > > Documentation/devicetree/bindings/drm/msm/gpu.txt | 51 ++++++++++++++++++++ > Documentation/devicetree/bindings/drm/msm/hdmi.txt | 43 +++++++++++++++++ > Documentation/devicetree/bindings/drm/msm/msm.txt | 40 ++++++++++++++++ > drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 2 + > drivers/gpu/drm/msm/hdmi/hdmi.c | 55 ++++++++++++++-------- > drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 10 ++-- > drivers/gpu/drm/msm/msm_drv.c | 29 ++++++++++-- > 7 files changed, 204 insertions(+), 26 deletions(-) > create mode 100644 Documentation/devicetree/bindings/drm/msm/gpu.txt > create mode 100644 Documentation/devicetree/bindings/drm/msm/hdmi.txt > create mode 100644 Documentation/devicetree/bindings/drm/msm/msm.txt > > diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt b/Documentation/devicetree/bindings/drm/msm/gpu.txt > new file mode 100644 > index 0000000..6e33efe > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt > @@ -0,0 +1,51 @@ > +Qualcomm adreno/snapdragon GPU > + > +Required properties: > +- compatible: "qcom,adreno-3xx" As Olof said, choose a definite name here, and have variants claim compatibility with that in addition to a variant specific string. > +- reg: Physical base address and length of the controller's registers. Just the one reg entry? The example has reg-names. Either document the name or drop it. > +- interrupts: The interrupt outputs from the controller. How many? Which ones? Names? > +- clocks: device clocks > + See ../clocks/clock-bindings.txt for details. Similarly? I should be able to read the binding and put together a DTS. Currently I cannot. > +- qcom,chipid: gpu chip-id. Note this may become optional for future > + devices if we can reliably read the chipid from hw What's the problem with reading chipid from HW at the moment? Should this possibly be optional, and only if not possible to read from HW? > +- qcom,gpu-pwrlevels: array of OPPs, sorted highest to lowest This is confusing. This is a node rather than a property, and in general it's not a good idea to rely on node ordering (there's no such thing as an array of nodes in the DTB). > + - compatible: "qcom,gpu-pwrlevels" > + - for each qcom,gpu-pwrlevel: Is that a compatible string for each node, name, or what? Any reason for having this as a container node at all? > + - qcom,gpu-freq: requested gpu clock speed > + - NOTE: downstream android driver defines additional parameters to > + configure memory bandwidth scaling per OPP. So? Either define those or don't bother. Having a halfway house binding is not helpful. > + > +Optional properties: > +- n/a Drop this if there are no optional properties. > + > +Example: > + > +/ { > + ... > + > + gpu: qcom,kgsl-3d0@4300000 { > + compatible = "qcom,adreno-3xx"; > + reg = <0x04300000 0x20000>; > + reg-names = "kgsl_3d0_reg_memory"; > + interrupts = <GIC_SPI 80 0>; > + interrupt-names = "kgsl_3d0_irq"; > + clock-names = > + "core_clk", > + "iface_clk", > + "mem_iface_clk"; > + clocks = > + <&mmcc GFX3D_CLK>, > + <&mmcc GFX3D_AHB_CLK>, > + <&mmcc MMSS_IMEM_AHB_CLK>; > + qcom,chipid = <0x03020100>; > + qcom,gpu-pwrlevels { > + compatible = "qcom,gpu-pwrlevels"; > + qcom,gpu-pwrlevel@0 { > + qcom,gpu-freq = <450000000>; > + }; > + qcom,gpu-pwrlevel@1 { > + qcom,gpu-freq = <27000000>; > + }; > + }; > + }; > +}; > diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > new file mode 100644 > index 0000000..051a49f > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > @@ -0,0 +1,43 @@ > +Qualcomm adreno/snapdragon hdmi output > + > +Required properties: > +- compatible: "qcom,hdmi-tx-8x60", "qcom,hdmi-tx-8960", "qcom,hdmi-tx-8x74" Please don't use wildcard strings. Is a node expected to have one of these entries, or all? I would recommend using a bulleted list to describe compatible strings. It allows for easier extension and the easy addition of notes. e.g. - compatible: should contain: * "foo,bar-xtreme" for bar variants with the xtreme extensions. * "foo,bar" for all bar variants (inc. xtreme). * "foo,baz" for baz variants. * "foo,foo" for all variants. > +- reg: Physical base address and length of the controller's registers. > +- interrupts: The interrupt outputs from the controller. > +- clocks: device clocks One entry? Multiple? What do they correspond to on the data sheet? Names? > + See ../clocks/clock-bindings.txt for details. > +- qcom,hdmi-tx-ddc-clk: ddc clk pin > +- qcom,hdmi-tx-ddc-data: ddc data pin > +- qcom,hdmi-tx-hpd: hpd pin GPIOs should be called *-gpios. > +- core-vdda-supply: phandle to supply regulator > +- hdmi-mux-supply: phandle to mux regulator > + > +Optional properties: > +- qcom,hdmi-tx-mux-en: hdmi mux enable pin > +- qcom,hdmi-tx-mux-sel: hdmi mux select pin GPIOs again. > + > +Example: > + > +/ { > + ... > + > + hdmi: qcom,hdmi-tx-8960@4a00000 { > + compatible = "qcom,hdmi-tx-8960"; > + reg-names = "core_physical"; > + reg = <0x04a00000 0x1000>; > + interrupts = <GIC_SPI 79 0>; > + clock-names = > + "core_clk", > + "master_iface_clk", > + "slave_iface_clk"; > + clocks = > + <&mmcc HDMI_APP_CLK>, > + <&mmcc HDMI_M_AHB_CLK>, > + <&mmcc HDMI_S_AHB_CLK>; > + qcom,hdmi-tx-ddc-clk = <&msmgpio 70 GPIO_ACTIVE_HIGH>; > + qcom,hdmi-tx-ddc-data = <&msmgpio 71 GPIO_ACTIVE_HIGH>; > + qcom,hdmi-tx-hpd = <&msmgpio 72 GPIO_ACTIVE_HIGH>; > + core-vdda-supply = <&pm8921_hdmi_mvs>; > + hdmi-mux-supply = <&ext_3p3v>; > + }; > +}; > diff --git a/Documentation/devicetree/bindings/drm/msm/msm.txt b/Documentation/devicetree/bindings/drm/msm/msm.txt > new file mode 100644 > index 0000000..484cc12 > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/msm/msm.txt > @@ -0,0 +1,40 @@ > +Qualcomm adreno/snapdragon What exactly is this unit? Judging by the GPU phandle it's not the GPU itself. > + > +Required properties: > +- compatible: "qcom,mdp" (mdp4) or "qcom,mdss_mdp" (mdp5) > +- reg: Physical base address and length of the controller's registers. > +- interrupts: The interrupt outputs from the controller. All of my prior points for these properties stand. > +- connectors: array of phandles for output device(s) What are valid devices to point this at? > +- clocks: device clocks Likewise, see my above points regarding clocks. > + See ../clocks/clock-bindings.txt for details. > + > +Optional properties: > +- gpus: phandle for gpu device What exactly is this used for? Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 2, 2014 at 1:12 PM, Olof Johansson <olof@lixom.net> wrote: > On Wed, Jul 2, 2014 at 7:26 AM, Jordan Crouse <jcrouse@codeaurora.org> wrote: >> On 07/01/2014 12:57 PM, Rob Clark wrote: >>> >>> diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt >>> b/Documentation/devicetree/bindings/drm/msm/gpu.txt >>> new file mode 100644 >>> index 0000000..6e33efe >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt >>> @@ -0,0 +1,51 @@ >>> +Qualcomm adreno/snapdragon GPU >>> + >>> +Required properties: >>> +- compatible: "qcom,adreno-3xx" >> >> >> 3xx won't always be the only game in town. Not that I have a better name, >> just pointing out that it has a limited shelf life. > > Actually, we prefer to avoid "xx" compatible strings. The compatible > should be the first actual device that is part of the xx series, and > newer IP should claim to be compatible with their own version plus the > original one (if they are possible to use with the same exact > programming model and only need extra code to deal with extensions). > > That way you'll also have a way to tell them apart in the driver if > you can't derive it out of register contents, etc. well, we could put entire chip-id (including the patch revision) in the compatible string.. ie. qcom,adreno-320.2 BR, -R > > -Olof -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 2, 2014 at 2:09 PM, Mark Rutland <mark.rutland@arm.com> wrote: > On Tue, Jul 01, 2014 at 07:57:35PM +0100, Rob Clark wrote: >> Now that we (almost) have enough dependencies in place (MMCC, RPM, etc), >> add necessary DT support so that we can use drm/msm on upstream kernel. >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> >> --- >> Commence bikeshedding :-) >> >> Documentation/devicetree/bindings/drm/msm/gpu.txt | 51 ++++++++++++++++++++ >> Documentation/devicetree/bindings/drm/msm/hdmi.txt | 43 +++++++++++++++++ >> Documentation/devicetree/bindings/drm/msm/msm.txt | 40 ++++++++++++++++ >> drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 2 + >> drivers/gpu/drm/msm/hdmi/hdmi.c | 55 ++++++++++++++-------- >> drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 10 ++-- >> drivers/gpu/drm/msm/msm_drv.c | 29 ++++++++++-- >> 7 files changed, 204 insertions(+), 26 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/drm/msm/gpu.txt >> create mode 100644 Documentation/devicetree/bindings/drm/msm/hdmi.txt >> create mode 100644 Documentation/devicetree/bindings/drm/msm/msm.txt >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt b/Documentation/devicetree/bindings/drm/msm/gpu.txt >> new file mode 100644 >> index 0000000..6e33efe >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt >> @@ -0,0 +1,51 @@ >> +Qualcomm adreno/snapdragon GPU >> + >> +Required properties: >> +- compatible: "qcom,adreno-3xx" > > As Olof said, choose a definite name here, and have variants claim > compatibility with that in addition to a variant specific string. > >> +- reg: Physical base address and length of the controller's registers. > > Just the one reg entry? as far as I know.. keep in mind, I'm working without docs > The example has reg-names. Either document the name or drop it. > >> +- interrupts: The interrupt outputs from the controller. > > How many? Which ones? Names? > >> +- clocks: device clocks >> + See ../clocks/clock-bindings.txt for details. > > Similarly? > > I should be able to read the binding and put together a DTS. Currently I > cannot. > >> +- qcom,chipid: gpu chip-id. Note this may become optional for future >> + devices if we can reliably read the chipid from hw > > What's the problem with reading chipid from HW at the moment? > > Should this possibly be optional, and only if not possible to read from > HW? As far as I can tell from diving downstream android kgsl code, seems like some a2xx you might be able to read the value from hw. Beyond that I'm not entirely sure. (Remember, no docs.) I'm leaving it as-is and if someone who has docs wants to submit patch to change it, that is fine. >> +- qcom,gpu-pwrlevels: array of OPPs, sorted highest to lowest > > This is confusing. This is a node rather than a property, and in general > it's not a good idea to rely on node ordering (there's no such thing as > an array of nodes in the DTB). > >> + - compatible: "qcom,gpu-pwrlevels" >> + - for each qcom,gpu-pwrlevel: > > Is that a compatible string for each node, name, or what? > > Any reason for having this as a container node at all? some of the decisions are made to make it easier to re-use/support existing bindings in downstream kernel.. >> + - qcom,gpu-freq: requested gpu clock speed >> + - NOTE: downstream android driver defines additional parameters to >> + configure memory bandwidth scaling per OPP. > > So? Either define those or don't bother. Having a halfway house binding > is not helpful. I'm not really in a position to document them at this point, sorry. >> + >> +Optional properties: >> +- n/a > > Drop this if there are no optional properties. > >> + >> +Example: >> + >> +/ { >> + ... >> + >> + gpu: qcom,kgsl-3d0@4300000 { >> + compatible = "qcom,adreno-3xx"; >> + reg = <0x04300000 0x20000>; >> + reg-names = "kgsl_3d0_reg_memory"; >> + interrupts = <GIC_SPI 80 0>; >> + interrupt-names = "kgsl_3d0_irq"; >> + clock-names = >> + "core_clk", >> + "iface_clk", >> + "mem_iface_clk"; >> + clocks = >> + <&mmcc GFX3D_CLK>, >> + <&mmcc GFX3D_AHB_CLK>, >> + <&mmcc MMSS_IMEM_AHB_CLK>; >> + qcom,chipid = <0x03020100>; >> + qcom,gpu-pwrlevels { >> + compatible = "qcom,gpu-pwrlevels"; >> + qcom,gpu-pwrlevel@0 { >> + qcom,gpu-freq = <450000000>; >> + }; >> + qcom,gpu-pwrlevel@1 { >> + qcom,gpu-freq = <27000000>; >> + }; >> + }; >> + }; >> +}; >> diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> new file mode 100644 >> index 0000000..051a49f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> @@ -0,0 +1,43 @@ >> +Qualcomm adreno/snapdragon hdmi output >> + >> +Required properties: >> +- compatible: "qcom,hdmi-tx-8x60", "qcom,hdmi-tx-8960", "qcom,hdmi-tx-8x74" > > Please don't use wildcard strings. > > Is a node expected to have one of these entries, or all? not quite sure what you mean.. Yes a node is expected to have 'compatible'. Which should have one of those values (with potentially more values added later) > I would recommend using a bulleted list to describe compatible strings. > It allows for easier extension and the easy addition of notes. e.g. > > - compatible: should contain: > * "foo,bar-xtreme" for bar variants with the xtreme extensions. > * "foo,bar" for all bar variants (inc. xtreme). > * "foo,baz" for baz variants. > * "foo,foo" for all variants. sure >> +- reg: Physical base address and length of the controller's registers. >> +- interrupts: The interrupt outputs from the controller. >> +- clocks: device clocks > > One entry? Multiple? > > What do they correspond to on the data sheet? get someone to send me a data sheet. I'd love to tell you then ;-) > Names? > >> + See ../clocks/clock-bindings.txt for details. >> +- qcom,hdmi-tx-ddc-clk: ddc clk pin >> +- qcom,hdmi-tx-ddc-data: ddc data pin >> +- qcom,hdmi-tx-hpd: hpd pin > > GPIOs should be called *-gpios. again just trying to be compatible with some existing downstream DT.. >> +- core-vdda-supply: phandle to supply regulator >> +- hdmi-mux-supply: phandle to mux regulator >> + >> +Optional properties: >> +- qcom,hdmi-tx-mux-en: hdmi mux enable pin >> +- qcom,hdmi-tx-mux-sel: hdmi mux select pin > > GPIOs again. > >> + >> +Example: >> + >> +/ { >> + ... >> + >> + hdmi: qcom,hdmi-tx-8960@4a00000 { >> + compatible = "qcom,hdmi-tx-8960"; >> + reg-names = "core_physical"; >> + reg = <0x04a00000 0x1000>; >> + interrupts = <GIC_SPI 79 0>; >> + clock-names = >> + "core_clk", >> + "master_iface_clk", >> + "slave_iface_clk"; >> + clocks = >> + <&mmcc HDMI_APP_CLK>, >> + <&mmcc HDMI_M_AHB_CLK>, >> + <&mmcc HDMI_S_AHB_CLK>; >> + qcom,hdmi-tx-ddc-clk = <&msmgpio 70 GPIO_ACTIVE_HIGH>; >> + qcom,hdmi-tx-ddc-data = <&msmgpio 71 GPIO_ACTIVE_HIGH>; >> + qcom,hdmi-tx-hpd = <&msmgpio 72 GPIO_ACTIVE_HIGH>; >> + core-vdda-supply = <&pm8921_hdmi_mvs>; >> + hdmi-mux-supply = <&ext_3p3v>; >> + }; >> +}; >> diff --git a/Documentation/devicetree/bindings/drm/msm/msm.txt b/Documentation/devicetree/bindings/drm/msm/msm.txt >> new file mode 100644 >> index 0000000..484cc12 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/msm/msm.txt >> @@ -0,0 +1,40 @@ >> +Qualcomm adreno/snapdragon > > What exactly is this unit? Judging by the GPU phandle it's not the GPU > itself. the display >> + >> +Required properties: >> +- compatible: "qcom,mdp" (mdp4) or "qcom,mdss_mdp" (mdp5) >> +- reg: Physical base address and length of the controller's registers. >> +- interrupts: The interrupt outputs from the controller. > > All of my prior points for these properties stand. > >> +- connectors: array of phandles for output device(s) > > What are valid devices to point this at? > >> +- clocks: device clocks > > Likewise, see my above points regarding clocks. > >> + See ../clocks/clock-bindings.txt for details. >> + >> +Optional properties: >> +- gpus: phandle for gpu device > > What exactly is this used for? 'gpus' and 'connectors' is basically part of the componentized device support. It is how the master/toplevel drm device knows what sub-devices exist. Currently there is just one valid possibility for each, but as we add DSI, DP, etc, and support for various other generations there will hopefully be more. (For example, some older snapdragons had one 3d core and zero to two 2d cores.) BR, -R > Thanks, > Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/02/2014 03:01 PM, Rob Clark wrote: > As far as I can tell from diving downstream android kgsl code, seems > like some a2xx you might be able to read the value from hw. Beyond > that I'm not entirely sure. (Remember, no docs.) Ohhh! I get to tell a story! The on chip registers have a long and consistent history of being wrong. And not just the spin ID being wrong - the major and minor have had their issues too. It got to the point that sometime around the A320 timeframe we put our foot down and decided that we would never again rely on the hardware. The reasoning is that the SoC and the GPU are on the same die so we can use the one to identify the other. And thats what we've done since then - first in the board files and now in the DT. So long story short - the registers are there, we don't trust them as far as we could throw them. Use the SoC instead. Jordan
On Wed, Jul 02, 2014 at 10:01:40PM +0100, Rob Clark wrote: > On Wed, Jul 2, 2014 at 2:09 PM, Mark Rutland <mark.rutland@arm.com> wrote: > > On Tue, Jul 01, 2014 at 07:57:35PM +0100, Rob Clark wrote: > >> Now that we (almost) have enough dependencies in place (MMCC, RPM, etc), > >> add necessary DT support so that we can use drm/msm on upstream kernel. > >> > >> Signed-off-by: Rob Clark <robdclark@gmail.com> > >> --- > >> Commence bikeshedding :-) > >> > >> Documentation/devicetree/bindings/drm/msm/gpu.txt | 51 ++++++++++++++++++++ > >> Documentation/devicetree/bindings/drm/msm/hdmi.txt | 43 +++++++++++++++++ > >> Documentation/devicetree/bindings/drm/msm/msm.txt | 40 ++++++++++++++++ > >> drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 2 + > >> drivers/gpu/drm/msm/hdmi/hdmi.c | 55 ++++++++++++++-------- > >> drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 10 ++-- > >> drivers/gpu/drm/msm/msm_drv.c | 29 ++++++++++-- > >> 7 files changed, 204 insertions(+), 26 deletions(-) > >> create mode 100644 Documentation/devicetree/bindings/drm/msm/gpu.txt > >> create mode 100644 Documentation/devicetree/bindings/drm/msm/hdmi.txt > >> create mode 100644 Documentation/devicetree/bindings/drm/msm/msm.txt > >> > >> diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt b/Documentation/devicetree/bindings/drm/msm/gpu.txt > >> new file mode 100644 > >> index 0000000..6e33efe > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt > >> @@ -0,0 +1,51 @@ > >> +Qualcomm adreno/snapdragon GPU > >> + > >> +Required properties: > >> +- compatible: "qcom,adreno-3xx" > > > > As Olof said, choose a definite name here, and have variants claim > > compatibility with that in addition to a variant specific string. > > > >> +- reg: Physical base address and length of the controller's registers. > > > > Just the one reg entry? > > as far as I know.. keep in mind, I'm working without docs Sure. If we only know about one reg entrty that's fine. > > > The example has reg-names. Either document the name or drop it. > > > >> +- interrupts: The interrupt outputs from the controller. > > > > How many? Which ones? Names? > > > >> +- clocks: device clocks > >> + See ../clocks/clock-bindings.txt for details. > > > > Similarly? > > > > I should be able to read the binding and put together a DTS. Currently I > > cannot. > > > >> +- qcom,chipid: gpu chip-id. Note this may become optional for future > >> + devices if we can reliably read the chipid from hw > > > > What's the problem with reading chipid from HW at the moment? > > > > Should this possibly be optional, and only if not possible to read from > > HW? > > As far as I can tell from diving downstream android kgsl code, seems > like some a2xx you might be able to read the value from hw. Beyond > that I'm not entirely sure. (Remember, no docs.) > > I'm leaving it as-is and if someone who has docs wants to submit patch > to change it, that is fine. My concern is that the statement "this may become optional" is useless, as it doesn't tell you what expected if it's not present. I would reword this as: - qcom,chip-id: The gpu chip-id, if the hardware value is not reliable. And then read from the HW if it's not present. > >> +- qcom,gpu-pwrlevels: array of OPPs, sorted highest to lowest > > > > This is confusing. This is a node rather than a property, and in general > > it's not a good idea to rely on node ordering (there's no such thing as > > an array of nodes in the DTB). > > > >> + - compatible: "qcom,gpu-pwrlevels" > >> + - for each qcom,gpu-pwrlevel: > > > > Is that a compatible string for each node, name, or what? > > > > Any reason for having this as a container node at all? > > some of the decisions are made to make it easier to re-use/support > existing bindings in downstream kernel.. I don't see why we should be beholden to downstream kernel bindings. > > >> + - qcom,gpu-freq: requested gpu clock speed > >> + - NOTE: downstream android driver defines additional parameters to > >> + configure memory bandwidth scaling per OPP. > > > > So? Either define those or don't bother. Having a halfway house binding > > is not helpful. > > I'm not really in a position to document them at this point, sorry. Then drop it. > >> + > >> +Optional properties: > >> +- n/a > > > > Drop this if there are no optional properties. > > > >> + > >> +Example: > >> + > >> +/ { > >> + ... > >> + > >> + gpu: qcom,kgsl-3d0@4300000 { > >> + compatible = "qcom,adreno-3xx"; > >> + reg = <0x04300000 0x20000>; > >> + reg-names = "kgsl_3d0_reg_memory"; > >> + interrupts = <GIC_SPI 80 0>; > >> + interrupt-names = "kgsl_3d0_irq"; > >> + clock-names = > >> + "core_clk", > >> + "iface_clk", > >> + "mem_iface_clk"; > >> + clocks = > >> + <&mmcc GFX3D_CLK>, > >> + <&mmcc GFX3D_AHB_CLK>, > >> + <&mmcc MMSS_IMEM_AHB_CLK>; > >> + qcom,chipid = <0x03020100>; > >> + qcom,gpu-pwrlevels { > >> + compatible = "qcom,gpu-pwrlevels"; > >> + qcom,gpu-pwrlevel@0 { > >> + qcom,gpu-freq = <450000000>; > >> + }; > >> + qcom,gpu-pwrlevel@1 { > >> + qcom,gpu-freq = <27000000>; > >> + }; > >> + }; > >> + }; > >> +}; > >> diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > >> new file mode 100644 > >> index 0000000..051a49f > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > >> @@ -0,0 +1,43 @@ > >> +Qualcomm adreno/snapdragon hdmi output > >> + > >> +Required properties: > >> +- compatible: "qcom,hdmi-tx-8x60", "qcom,hdmi-tx-8960", "qcom,hdmi-tx-8x74" > > > > Please don't use wildcard strings. > > > > Is a node expected to have one of these entries, or all? > > not quite sure what you mean.. Yes a node is expected to have > 'compatible'. Which should have one of those values (with potentially > more values added later) Sorry for the lack of clarity. I meant is it expected that I should have: compatible = "qcom,hdmi-tx-8x60", "qcom,hdmi-tx-8960", "qcom,hdmi-tx-8x74"; Or just a single string for a given device? I assume the latter. > > > I would recommend using a bulleted list to describe compatible strings. > > It allows for easier extension and the easy addition of notes. e.g. > > > > - compatible: should contain: > > * "foo,bar-xtreme" for bar variants with the xtreme extensions. > > * "foo,bar" for all bar variants (inc. xtreme). > > * "foo,baz" for baz variants. > > * "foo,foo" for all variants. > > sure > > >> +- reg: Physical base address and length of the controller's registers. > >> +- interrupts: The interrupt outputs from the controller. > >> +- clocks: device clocks > > > > One entry? Multiple? > > > > What do they correspond to on the data sheet? > > get someone to send me a data sheet. I'd love to tell you then ;-) Sure :) > > Names? > > > >> + See ../clocks/clock-bindings.txt for details. > >> +- qcom,hdmi-tx-ddc-clk: ddc clk pin > >> +- qcom,hdmi-tx-ddc-data: ddc data pin > >> +- qcom,hdmi-tx-hpd: hpd pin > > > > GPIOs should be called *-gpios. > > again just trying to be compatible with some existing downstream DT.. And again I'm not certain that's a good idea... > >> +- core-vdda-supply: phandle to supply regulator > >> +- hdmi-mux-supply: phandle to mux regulator > >> + > >> +Optional properties: > >> +- qcom,hdmi-tx-mux-en: hdmi mux enable pin > >> +- qcom,hdmi-tx-mux-sel: hdmi mux select pin > > > > GPIOs again. > > > >> + > >> +Example: > >> + > >> +/ { > >> + ... > >> + > >> + hdmi: qcom,hdmi-tx-8960@4a00000 { > >> + compatible = "qcom,hdmi-tx-8960"; > >> + reg-names = "core_physical"; > >> + reg = <0x04a00000 0x1000>; > >> + interrupts = <GIC_SPI 79 0>; > >> + clock-names = > >> + "core_clk", > >> + "master_iface_clk", > >> + "slave_iface_clk"; > >> + clocks = > >> + <&mmcc HDMI_APP_CLK>, > >> + <&mmcc HDMI_M_AHB_CLK>, > >> + <&mmcc HDMI_S_AHB_CLK>; > >> + qcom,hdmi-tx-ddc-clk = <&msmgpio 70 GPIO_ACTIVE_HIGH>; > >> + qcom,hdmi-tx-ddc-data = <&msmgpio 71 GPIO_ACTIVE_HIGH>; > >> + qcom,hdmi-tx-hpd = <&msmgpio 72 GPIO_ACTIVE_HIGH>; > >> + core-vdda-supply = <&pm8921_hdmi_mvs>; > >> + hdmi-mux-supply = <&ext_3p3v>; > >> + }; > >> +}; > >> diff --git a/Documentation/devicetree/bindings/drm/msm/msm.txt b/Documentation/devicetree/bindings/drm/msm/msm.txt > >> new file mode 100644 > >> index 0000000..484cc12 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/drm/msm/msm.txt > >> @@ -0,0 +1,40 @@ > >> +Qualcomm adreno/snapdragon > > > > What exactly is this unit? Judging by the GPU phandle it's not the GPU > > itself. > > the display I see. Could that be mentioned in the heading line above? > > >> + > >> +Required properties: > >> +- compatible: "qcom,mdp" (mdp4) or "qcom,mdss_mdp" (mdp5) > >> +- reg: Physical base address and length of the controller's registers. > >> +- interrupts: The interrupt outputs from the controller. > > > > All of my prior points for these properties stand. > > > >> +- connectors: array of phandles for output device(s) > > > > What are valid devices to point this at? > > > >> +- clocks: device clocks > > > > Likewise, see my above points regarding clocks. > > > >> + See ../clocks/clock-bindings.txt for details. > >> + > >> +Optional properties: > >> +- gpus: phandle for gpu device > > > > What exactly is this used for? > > 'gpus' and 'connectors' is basically part of the componentized device > support. It is how the master/toplevel drm device knows what > sub-devices exist. > > Currently there is just one valid possibility for each, but as we add > DSI, DP, etc, and support for various other generations there will > hopefully be more. (For example, some older snapdragons had one 3d > core and zero to two 2d cores.) Ok. I must admit to being unfamiliar with how that's expected to work. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 3, 2014 at 5:15 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Wed, Jul 02, 2014 at 10:01:40PM +0100, Rob Clark wrote: >> On Wed, Jul 2, 2014 at 2:09 PM, Mark Rutland <mark.rutland@arm.com> wrote: >> > On Tue, Jul 01, 2014 at 07:57:35PM +0100, Rob Clark wrote: >> >> Now that we (almost) have enough dependencies in place (MMCC, RPM, etc), >> >> add necessary DT support so that we can use drm/msm on upstream kernel. >> >> >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> >> >> --- >> >> Commence bikeshedding :-) >> >> >> >> Documentation/devicetree/bindings/drm/msm/gpu.txt | 51 ++++++++++++++++++++ >> >> Documentation/devicetree/bindings/drm/msm/hdmi.txt | 43 +++++++++++++++++ >> >> Documentation/devicetree/bindings/drm/msm/msm.txt | 40 ++++++++++++++++ >> >> drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 2 + >> >> drivers/gpu/drm/msm/hdmi/hdmi.c | 55 ++++++++++++++-------- >> >> drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 10 ++-- >> >> drivers/gpu/drm/msm/msm_drv.c | 29 ++++++++++-- >> >> 7 files changed, 204 insertions(+), 26 deletions(-) >> >> create mode 100644 Documentation/devicetree/bindings/drm/msm/gpu.txt >> >> create mode 100644 Documentation/devicetree/bindings/drm/msm/hdmi.txt >> >> create mode 100644 Documentation/devicetree/bindings/drm/msm/msm.txt >> >> >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/gpu.txt b/Documentation/devicetree/bindings/drm/msm/gpu.txt >> >> new file mode 100644 >> >> index 0000000..6e33efe >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt >> >> @@ -0,0 +1,51 @@ >> >> +Qualcomm adreno/snapdragon GPU >> >> + >> >> +Required properties: >> >> +- compatible: "qcom,adreno-3xx" >> > >> > As Olof said, choose a definite name here, and have variants claim >> > compatibility with that in addition to a variant specific string. >> > >> >> +- reg: Physical base address and length of the controller's registers. >> > >> > Just the one reg entry? >> >> as far as I know.. keep in mind, I'm working without docs > > Sure. If we only know about one reg entrty that's fine. > >> >> > The example has reg-names. Either document the name or drop it. >> > >> >> +- interrupts: The interrupt outputs from the controller. >> > >> > How many? Which ones? Names? >> > >> >> +- clocks: device clocks >> >> + See ../clocks/clock-bindings.txt for details. >> > >> > Similarly? >> > >> > I should be able to read the binding and put together a DTS. Currently I >> > cannot. >> > >> >> +- qcom,chipid: gpu chip-id. Note this may become optional for future >> >> + devices if we can reliably read the chipid from hw >> > >> > What's the problem with reading chipid from HW at the moment? >> > >> > Should this possibly be optional, and only if not possible to read from >> > HW? >> >> As far as I can tell from diving downstream android kgsl code, seems >> like some a2xx you might be able to read the value from hw. Beyond >> that I'm not entirely sure. (Remember, no docs.) >> >> I'm leaving it as-is and if someone who has docs wants to submit patch >> to change it, that is fine. > > My concern is that the statement "this may become optional" is useless, > as it doesn't tell you what expected if it's not present. > > I would reword this as: > > - qcom,chip-id: The gpu chip-id, if the hardware value is not reliable. yeah, ok. That is more clear. > And then read from the HW if it's not present. > >> >> +- qcom,gpu-pwrlevels: array of OPPs, sorted highest to lowest >> > >> > This is confusing. This is a node rather than a property, and in general >> > it's not a good idea to rely on node ordering (there's no such thing as >> > an array of nodes in the DTB). btw, we don't actually rely on the ordering in the parsing code. Although I think it is more readable if it is kept sorted. So I suppose consider that as more of a suggestion. >> > >> >> + - compatible: "qcom,gpu-pwrlevels" >> >> + - for each qcom,gpu-pwrlevel: >> > >> > Is that a compatible string for each node, name, or what? >> > >> > Any reason for having this as a container node at all? >> >> some of the decisions are made to make it easier to re-use/support >> existing bindings in downstream kernel.. > > I don't see why we should be beholden to downstream kernel bindings. Well, unfortunately since devices that run upstream kernel are few and far between, I get to port upstream driver to more downstream device kernels than I'd care to. To simplify this, as much as possible I'd like to share DT binding code. The alternative is more error prone and time consuming, and it's not exactly like I'm running out of things to do. >> >> >> + - qcom,gpu-freq: requested gpu clock speed >> >> + - NOTE: downstream android driver defines additional parameters to >> >> + configure memory bandwidth scaling per OPP. >> > >> > So? Either define those or don't bother. Having a halfway house binding >> > is not helpful. >> >> I'm not really in a position to document them at this point, sorry. > > Then drop it. Not sure I understand. The gpu-freq we need. I could drop the NOTE, although it does kind of explain why a simple list of integers is not used. Anything added later would be optional, ofc, for backwards compat. >> >> + >> >> +Optional properties: >> >> +- n/a >> > >> > Drop this if there are no optional properties. >> > >> >> + >> >> +Example: >> >> + >> >> +/ { >> >> + ... >> >> + >> >> + gpu: qcom,kgsl-3d0@4300000 { >> >> + compatible = "qcom,adreno-3xx"; >> >> + reg = <0x04300000 0x20000>; >> >> + reg-names = "kgsl_3d0_reg_memory"; >> >> + interrupts = <GIC_SPI 80 0>; >> >> + interrupt-names = "kgsl_3d0_irq"; >> >> + clock-names = >> >> + "core_clk", >> >> + "iface_clk", >> >> + "mem_iface_clk"; >> >> + clocks = >> >> + <&mmcc GFX3D_CLK>, >> >> + <&mmcc GFX3D_AHB_CLK>, >> >> + <&mmcc MMSS_IMEM_AHB_CLK>; >> >> + qcom,chipid = <0x03020100>; >> >> + qcom,gpu-pwrlevels { >> >> + compatible = "qcom,gpu-pwrlevels"; >> >> + qcom,gpu-pwrlevel@0 { >> >> + qcom,gpu-freq = <450000000>; >> >> + }; >> >> + qcom,gpu-pwrlevel@1 { >> >> + qcom,gpu-freq = <27000000>; >> >> + }; >> >> + }; >> >> + }; >> >> +}; >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> >> new file mode 100644 >> >> index 0000000..051a49f >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> >> @@ -0,0 +1,43 @@ >> >> +Qualcomm adreno/snapdragon hdmi output >> >> + >> >> +Required properties: >> >> +- compatible: "qcom,hdmi-tx-8x60", "qcom,hdmi-tx-8960", "qcom,hdmi-tx-8x74" >> > >> > Please don't use wildcard strings. >> > >> > Is a node expected to have one of these entries, or all? >> >> not quite sure what you mean.. Yes a node is expected to have >> 'compatible'. Which should have one of those values (with potentially >> more values added later) > > Sorry for the lack of clarity. I meant is it expected that I should > have: > > compatible = "qcom,hdmi-tx-8x60", "qcom,hdmi-tx-8960", > "qcom,hdmi-tx-8x74"; > > Or just a single string for a given device? > > I assume the latter. oh, yes. I kinda thought the use of "compatible" everywhere was enough that it would be obvious that this meant "pick one".. I guess re-arranging as a bullet-list would make that more clear. >> >> > I would recommend using a bulleted list to describe compatible strings. >> > It allows for easier extension and the easy addition of notes. e.g. >> > >> > - compatible: should contain: >> > * "foo,bar-xtreme" for bar variants with the xtreme extensions. >> > * "foo,bar" for all bar variants (inc. xtreme). >> > * "foo,baz" for baz variants. >> > * "foo,foo" for all variants. >> >> sure >> >> >> +- reg: Physical base address and length of the controller's registers. >> >> +- interrupts: The interrupt outputs from the controller. >> >> +- clocks: device clocks >> > >> > One entry? Multiple? >> > >> > What do they correspond to on the data sheet? >> >> get someone to send me a data sheet. I'd love to tell you then ;-) > > Sure :) > >> > Names? >> > >> >> + See ../clocks/clock-bindings.txt for details. >> >> +- qcom,hdmi-tx-ddc-clk: ddc clk pin >> >> +- qcom,hdmi-tx-ddc-data: ddc data pin >> >> +- qcom,hdmi-tx-hpd: hpd pin >> > >> > GPIOs should be called *-gpios. >> >> again just trying to be compatible with some existing downstream DT.. > > And again I'm not certain that's a good idea... > >> >> +- core-vdda-supply: phandle to supply regulator >> >> +- hdmi-mux-supply: phandle to mux regulator >> >> + >> >> +Optional properties: >> >> +- qcom,hdmi-tx-mux-en: hdmi mux enable pin >> >> +- qcom,hdmi-tx-mux-sel: hdmi mux select pin >> > >> > GPIOs again. >> > >> >> + >> >> +Example: >> >> + >> >> +/ { >> >> + ... >> >> + >> >> + hdmi: qcom,hdmi-tx-8960@4a00000 { >> >> + compatible = "qcom,hdmi-tx-8960"; >> >> + reg-names = "core_physical"; >> >> + reg = <0x04a00000 0x1000>; >> >> + interrupts = <GIC_SPI 79 0>; >> >> + clock-names = >> >> + "core_clk", >> >> + "master_iface_clk", >> >> + "slave_iface_clk"; >> >> + clocks = >> >> + <&mmcc HDMI_APP_CLK>, >> >> + <&mmcc HDMI_M_AHB_CLK>, >> >> + <&mmcc HDMI_S_AHB_CLK>; >> >> + qcom,hdmi-tx-ddc-clk = <&msmgpio 70 GPIO_ACTIVE_HIGH>; >> >> + qcom,hdmi-tx-ddc-data = <&msmgpio 71 GPIO_ACTIVE_HIGH>; >> >> + qcom,hdmi-tx-hpd = <&msmgpio 72 GPIO_ACTIVE_HIGH>; >> >> + core-vdda-supply = <&pm8921_hdmi_mvs>; >> >> + hdmi-mux-supply = <&ext_3p3v>; >> >> + }; >> >> +}; >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/msm.txt b/Documentation/devicetree/bindings/drm/msm/msm.txt >> >> new file mode 100644 >> >> index 0000000..484cc12 >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/drm/msm/msm.txt >> >> @@ -0,0 +1,40 @@ >> >> +Qualcomm adreno/snapdragon >> > >> > What exactly is this unit? Judging by the GPU phandle it's not the GPU >> > itself. >> >> the display > > I see. Could that be mentioned in the heading line above? will do >> >> >> + >> >> +Required properties: >> >> +- compatible: "qcom,mdp" (mdp4) or "qcom,mdss_mdp" (mdp5) >> >> +- reg: Physical base address and length of the controller's registers. >> >> +- interrupts: The interrupt outputs from the controller. >> > >> > All of my prior points for these properties stand. >> > >> >> +- connectors: array of phandles for output device(s) >> > >> > What are valid devices to point this at? >> > >> >> +- clocks: device clocks >> > >> > Likewise, see my above points regarding clocks. >> > >> >> + See ../clocks/clock-bindings.txt for details. >> >> + >> >> +Optional properties: >> >> +- gpus: phandle for gpu device >> > >> > What exactly is this used for? >> >> 'gpus' and 'connectors' is basically part of the componentized device >> support. It is how the master/toplevel drm device knows what >> sub-devices exist. >> >> Currently there is just one valid possibility for each, but as we add >> DSI, DP, etc, and support for various other generations there will >> hopefully be more. (For example, some older snapdragons had one 3d >> core and zero to two 2d cores.) > > Ok. I must admit to being unfamiliar with how that's expected to work. in case you are curious: http://lists.freedesktop.org/archives/dri-devel/2014-January/051249.html BR, -R > Thanks, > Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, > Now that we (almost) have enough dependencies in place (MMCC, RPM, etc), > add necessary DT support so that we can use drm/msm on upstream kernel. > > Signed-off-by: Rob Clark <robdclark@gmail.com> > --- > Commence bikeshedding :-) > <snip> > diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt > b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > new file mode 100644 > index 0000000..051a49f > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt > @@ -0,0 +1,43 @@ > +Qualcomm adreno/snapdragon hdmi output > + > +Required properties: > +- compatible: "qcom,hdmi-tx-8x60", "qcom,hdmi-tx-8960", > "qcom,hdmi-tx-8x74" > +- reg: Physical base address and length of the controller's registers. Since you are adding "qcom,hdmi-tx-8x74" (separate address space for PHY registers) in the compatible entry, how about this for the register description: - reg: Physical base address and length of the controllers' registers. - reg-names: names corresponding to the defined register sets, - "core_physical": HDMI Core registers - (optional) "phy_physical": HDMI PHY registers > +- interrupts: The interrupt outputs from the controller. > +- clocks: device clocks > + See ../clocks/clock-bindings.txt for details. > +- qcom,hdmi-tx-ddc-clk: ddc clk pin > +- qcom,hdmi-tx-ddc-data: ddc data pin > +- qcom,hdmi-tx-hpd: hpd pin > +- core-vdda-supply: phandle to supply regulator > +- hdmi-mux-supply: phandle to mux regulator > + > +Optional properties: > +- qcom,hdmi-tx-mux-en: hdmi mux enable pin > +- qcom,hdmi-tx-mux-sel: hdmi mux select pin > + > +Example: > + > +/ { > + ... > + > + hdmi: qcom,hdmi-tx-8960@4a00000 { > + compatible = "qcom,hdmi-tx-8960"; > + reg-names = "core_physical"; > + reg = <0x04a00000 0x1000>; > + interrupts = <GIC_SPI 79 0>; > + clock-names = > + "core_clk", > + "master_iface_clk", > + "slave_iface_clk"; > + clocks = > + <&mmcc HDMI_APP_CLK>, > + <&mmcc HDMI_M_AHB_CLK>, > + <&mmcc HDMI_S_AHB_CLK>; > + qcom,hdmi-tx-ddc-clk = <&msmgpio 70 GPIO_ACTIVE_HIGH>; > + qcom,hdmi-tx-ddc-data = <&msmgpio 71 GPIO_ACTIVE_HIGH>; > + qcom,hdmi-tx-hpd = <&msmgpio 72 GPIO_ACTIVE_HIGH>; > + core-vdda-supply = <&pm8921_hdmi_mvs>; > + hdmi-mux-supply = <&ext_3p3v>; > + }; > +}; > diff --git a/Documentation/devicetree/bindings/drm/msm/msm.txt b/Documentation/devicetree/bindings/drm/msm/msm.txt > new file mode 100644 > index 0000000..484cc12 > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/msm/msm.txt > <at> <at> -0,0 +1,40 <at> <at> > +Qualcomm adreno/snapdragon > + > +Required properties: > +- compatible: "qcom,mdp" (mdp4) or "qcom,mdss_mdp" (mdp5) > +- reg: Physical base address and length of the controller's registers. As per the code (mdp5_kms.c), there are two sets of registers: "mdp_phys" and "vbif_phys". They should probably be added in the description here. "reg-names" entry might be needed as well. > +- interrupts: The interrupt outputs from the controller. > +- connectors: array of phandles for output device(s) > +- clocks: device clocks > + See ../clocks/clock-bindings.txt for details. > + > +Optional properties: > +- gpus: phandle for gpu device > + > +Example: > + > +/ { > + ... > + > + mdp: qcom,mdp <at> 5100000 { > + compatible = "qcom,mdp"; > + reg = <0x05100000 0xf0000>; > + interrupts = <GIC_SPI 75 0>; > + connectors = <&hdmi>; > + gpus = <&gpu>; > + clock-names = > + "core_clk", > + "iface_clk", > + "lut_clk", > + "src_clk", > + "hdmi_clk", > + "mdp_clk"; > + clocks = > + <&mmcc MDP_SRC>, > + <&mmcc MDP_AHB_CLK>, > + <&mmcc MDP_LUT_CLK>, > + <&mmcc TV_SRC>, > + <&mmcc HDMI_TV_CLK>, > + <&mmcc MDP_TV_CLK>; > + }; > +}; <snip> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c > index 7f7aade..0ff8d46 100644 > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c <snip> > <at> <at> -273,24 +275,37 <at> <at> static int hdmi_bind(struct device *dev, struct device *master, void *data) > return gpio; > } > > - /* TODO actually use DT.. */ > - static const char *hpd_reg_names[] = {"hpd-gdsc", "hpd-5v"}; > - static const char *pwr_reg_names[] = {"core-vdda", "core-vcc"}; > - static const char *hpd_clk_names[] = {"iface_clk", "core_clk", "mdp_core_clk"}; > - static unsigned long hpd_clk_freq[] = {0, 19200000, 0}; > - static const char *pwr_clk_names[] = {"extp_clk", "alt_iface_clk"}; > + if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8x74")) { > + static const char *hpd_reg_names[] = {"hpd-gdsc", "hpd-5v"}; > + static const char *pwr_reg_names[] = {"core-vdda", "core-vcc"}; > + static const char *hpd_clk_names[] = {"iface_clk", "core_clk", "mdp_core_clk"}; > + static unsigned long hpd_clk_freq[] = {0, 19200000, 0}; > + static const char *pwr_clk_names[] = {"extp_clk", "alt_iface_clk"}; > + config.phy_init = hdmi_phy_8x74_init; > + config.hpd_reg_names = hpd_reg_names; > + config.hpd_reg_cnt = ARRAY_SIZE(hpd_reg_names); > + config.pwr_reg_names = pwr_reg_names; > + config.pwr_reg_cnt = ARRAY_SIZE(pwr_reg_names); > + config.hpd_clk_names = hpd_clk_names; > + config.hpd_freq = hpd_clk_freq; > + config.hpd_clk_cnt = ARRAY_SIZE(hpd_clk_names); > + config.pwr_clk_names = pwr_clk_names; > + config.pwr_clk_cnt = ARRAY_SIZE(pwr_clk_names); > + } else if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8960")) { > + static const char *hpd_clk_names[] = {"core_clk", "master_iface_clk", "slave_iface_clk"}; > + static const char *hpd_reg_names[] = {"core-vdda", "hdmi-mux"}; > + config.phy_init = hdmi_phy_8960_init; > + config.hpd_reg_names = hpd_reg_names; > + config.hpd_reg_cnt = ARRAY_SIZE(hpd_reg_names); > + config.hpd_clk_names = hpd_clk_names; > + config.hpd_clk_cnt = ARRAY_SIZE(hpd_clk_names); > + } else if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8x60")) { > + config.phy_init = hdmi_phy_8x60_init; > + } else { > + dev_err(dev, "unknown phy: %s\n", of_node->name); > + } Stéphane. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 3, 2014 at 12:36 PM, <sviau@codeaurora.org> wrote: > Hi Rob, > >> Now that we (almost) have enough dependencies in place (MMCC, RPM, etc), >> add necessary DT support so that we can use drm/msm on upstream kernel. >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> >> --- >> Commence bikeshedding :-) >> > <snip> >> diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> new file mode 100644 >> index 0000000..051a49f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> @@ -0,0 +1,43 @@ >> +Qualcomm adreno/snapdragon hdmi output >> + >> +Required properties: >> +- compatible: "qcom,hdmi-tx-8x60", "qcom,hdmi-tx-8960", >> "qcom,hdmi-tx-8x74" >> +- reg: Physical base address and length of the controller's registers. > > Since you are adding "qcom,hdmi-tx-8x74" (separate address space for PHY > registers) in the compatible entry, how about this for the register > description: > - reg: Physical base address and length of the controllers' registers. > - reg-names: names corresponding to the defined register sets, > - "core_physical": HDMI Core registers > - (optional) "phy_physical": HDMI PHY registers Oh, right.. I forgot 8x74 had phy in a separate region.. >> +- interrupts: The interrupt outputs from the controller. >> +- clocks: device clocks >> + See ../clocks/clock-bindings.txt for details. >> +- qcom,hdmi-tx-ddc-clk: ddc clk pin >> +- qcom,hdmi-tx-ddc-data: ddc data pin >> +- qcom,hdmi-tx-hpd: hpd pin >> +- core-vdda-supply: phandle to supply regulator >> +- hdmi-mux-supply: phandle to mux regulator >> + >> +Optional properties: >> +- qcom,hdmi-tx-mux-en: hdmi mux enable pin >> +- qcom,hdmi-tx-mux-sel: hdmi mux select pin >> + >> +Example: >> + >> +/ { >> + ... >> + >> + hdmi: qcom,hdmi-tx-8960@4a00000 { >> + compatible = "qcom,hdmi-tx-8960"; >> + reg-names = "core_physical"; >> + reg = <0x04a00000 0x1000>; >> + interrupts = <GIC_SPI 79 0>; >> + clock-names = >> + "core_clk", >> + "master_iface_clk", >> + "slave_iface_clk"; >> + clocks = >> + <&mmcc HDMI_APP_CLK>, >> + <&mmcc HDMI_M_AHB_CLK>, >> + <&mmcc HDMI_S_AHB_CLK>; >> + qcom,hdmi-tx-ddc-clk = <&msmgpio 70 GPIO_ACTIVE_HIGH>; >> + qcom,hdmi-tx-ddc-data = <&msmgpio 71 GPIO_ACTIVE_HIGH>; >> + qcom,hdmi-tx-hpd = <&msmgpio 72 GPIO_ACTIVE_HIGH>; >> + core-vdda-supply = <&pm8921_hdmi_mvs>; >> + hdmi-mux-supply = <&ext_3p3v>; >> + }; >> +}; >> diff --git a/Documentation/devicetree/bindings/drm/msm/msm.txt > b/Documentation/devicetree/bindings/drm/msm/msm.txt >> new file mode 100644 >> index 0000000..484cc12 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/msm/msm.txt >> <at> <at> -0,0 +1,40 <at> <at> >> +Qualcomm adreno/snapdragon >> + >> +Required properties: >> +- compatible: "qcom,mdp" (mdp4) or "qcom,mdss_mdp" (mdp5) >> +- reg: Physical base address and length of the controller's registers. > > As per the code (mdp5_kms.c), there are two sets of registers: "mdp_phys" > and "vbif_phys". They should probably be added in the description here. > "reg-names" entry might be needed as well. hmm.. well, we aren't actually using vbif_phys *yet*.. although not really sure if that will change. I am wondering if maybe we should split out mdp4 vs mdp5 DT bindings. Or at least remove the mdp5 compat string for now. So far I've only got mdp4 working upstream, so probably a few things I've missed for mdp5. Maybe just removing "qcom,mdss_mdp" for the time being is safer until we decide. I guess we should be really close to having all the needed dependencies for 8074/dragonboard, so I suppose that is something I should play with. Although seems unlikely that I'll have time until after 3.17 merge window. Mostly I wanted to get mdp4 working first, since I already have enough overlay support working there to be useful for atomic modeset/pageflip ;-) BR, -R >> +- interrupts: The interrupt outputs from the controller. >> +- connectors: array of phandles for output device(s) >> +- clocks: device clocks >> + See ../clocks/clock-bindings.txt for details. >> + >> +Optional properties: >> +- gpus: phandle for gpu device >> + >> +Example: >> + >> +/ { >> + ... >> + >> + mdp: qcom,mdp <at> 5100000 { >> + compatible = "qcom,mdp"; >> + reg = <0x05100000 0xf0000>; >> + interrupts = <GIC_SPI 75 0>; >> + connectors = <&hdmi>; >> + gpus = <&gpu>; >> + clock-names = >> + "core_clk", >> + "iface_clk", >> + "lut_clk", >> + "src_clk", >> + "hdmi_clk", >> + "mdp_clk"; >> + clocks = >> + <&mmcc MDP_SRC>, >> + <&mmcc MDP_AHB_CLK>, >> + <&mmcc MDP_LUT_CLK>, >> + <&mmcc TV_SRC>, >> + <&mmcc HDMI_TV_CLK>, >> + <&mmcc MDP_TV_CLK>; >> + }; >> +}; > <snip> >> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c > b/drivers/gpu/drm/msm/hdmi/hdmi.c >> index 7f7aade..0ff8d46 100644 >> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c >> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c > <snip> >> <at> <at> -273,24 +275,37 <at> <at> static int hdmi_bind(struct > device *dev, struct device *master, void *data) >> return gpio; >> } >> >> - /* TODO actually use DT.. */ >> - static const char *hpd_reg_names[] = {"hpd-gdsc", "hpd-5v"}; >> - static const char *pwr_reg_names[] = {"core-vdda", "core-vcc"}; >> - static const char *hpd_clk_names[] = {"iface_clk", "core_clk", > "mdp_core_clk"}; >> - static unsigned long hpd_clk_freq[] = {0, 19200000, 0}; >> - static const char *pwr_clk_names[] = {"extp_clk", "alt_iface_clk"}; >> + if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8x74")) { >> + static const char *hpd_reg_names[] = {"hpd-gdsc", "hpd-5v"}; >> + static const char *pwr_reg_names[] = {"core-vdda", "core-vcc"}; >> + static const char *hpd_clk_names[] = {"iface_clk", "core_clk", > "mdp_core_clk"}; >> + static unsigned long hpd_clk_freq[] = {0, 19200000, 0}; >> + static const char *pwr_clk_names[] = {"extp_clk", "alt_iface_clk"}; >> + config.phy_init = hdmi_phy_8x74_init; >> + config.hpd_reg_names = hpd_reg_names; >> + config.hpd_reg_cnt = ARRAY_SIZE(hpd_reg_names); >> + config.pwr_reg_names = pwr_reg_names; >> + config.pwr_reg_cnt = ARRAY_SIZE(pwr_reg_names); >> + config.hpd_clk_names = hpd_clk_names; >> + config.hpd_freq = hpd_clk_freq; >> + config.hpd_clk_cnt = ARRAY_SIZE(hpd_clk_names); >> + config.pwr_clk_names = pwr_clk_names; >> + config.pwr_clk_cnt = ARRAY_SIZE(pwr_clk_names); >> + } else if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8960")) { >> + static const char *hpd_clk_names[] = {"core_clk", "master_iface_clk", > "slave_iface_clk"}; >> + static const char *hpd_reg_names[] = {"core-vdda", "hdmi-mux"}; >> + config.phy_init = hdmi_phy_8960_init; >> + config.hpd_reg_names = hpd_reg_names; >> + config.hpd_reg_cnt = ARRAY_SIZE(hpd_reg_names); >> + config.hpd_clk_names = hpd_clk_names; >> + config.hpd_clk_cnt = ARRAY_SIZE(hpd_clk_names); >> + } else if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8x60")) { >> + config.phy_init = hdmi_phy_8x60_init; >> + } else { >> + dev_err(dev, "unknown phy: %s\n", of_node->name); >> + } > > Stéphane. > -- To unsubscribe from this list: send the line "unsubscribe devicetree" 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/msm/gpu.txt b/Documentation/devicetree/bindings/drm/msm/gpu.txt new file mode 100644 index 0000000..6e33efe --- /dev/null +++ b/Documentation/devicetree/bindings/drm/msm/gpu.txt @@ -0,0 +1,51 @@ +Qualcomm adreno/snapdragon GPU + +Required properties: +- compatible: "qcom,adreno-3xx" +- reg: Physical base address and length of the controller's registers. +- interrupts: The interrupt outputs from the controller. +- clocks: device clocks + See ../clocks/clock-bindings.txt for details. +- qcom,chipid: gpu chip-id. Note this may become optional for future + devices if we can reliably read the chipid from hw +- qcom,gpu-pwrlevels: array of OPPs, sorted highest to lowest + - compatible: "qcom,gpu-pwrlevels" + - for each qcom,gpu-pwrlevel: + - qcom,gpu-freq: requested gpu clock speed + - NOTE: downstream android driver defines additional parameters to + configure memory bandwidth scaling per OPP. + +Optional properties: +- n/a + +Example: + +/ { + ... + + gpu: qcom,kgsl-3d0@4300000 { + compatible = "qcom,adreno-3xx"; + reg = <0x04300000 0x20000>; + reg-names = "kgsl_3d0_reg_memory"; + interrupts = <GIC_SPI 80 0>; + interrupt-names = "kgsl_3d0_irq"; + clock-names = + "core_clk", + "iface_clk", + "mem_iface_clk"; + clocks = + <&mmcc GFX3D_CLK>, + <&mmcc GFX3D_AHB_CLK>, + <&mmcc MMSS_IMEM_AHB_CLK>; + qcom,chipid = <0x03020100>; + qcom,gpu-pwrlevels { + compatible = "qcom,gpu-pwrlevels"; + qcom,gpu-pwrlevel@0 { + qcom,gpu-freq = <450000000>; + }; + qcom,gpu-pwrlevel@1 { + qcom,gpu-freq = <27000000>; + }; + }; + }; +}; diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt new file mode 100644 index 0000000..051a49f --- /dev/null +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt @@ -0,0 +1,43 @@ +Qualcomm adreno/snapdragon hdmi output + +Required properties: +- compatible: "qcom,hdmi-tx-8x60", "qcom,hdmi-tx-8960", "qcom,hdmi-tx-8x74" +- reg: Physical base address and length of the controller's registers. +- interrupts: The interrupt outputs from the controller. +- clocks: device clocks + See ../clocks/clock-bindings.txt for details. +- qcom,hdmi-tx-ddc-clk: ddc clk pin +- qcom,hdmi-tx-ddc-data: ddc data pin +- qcom,hdmi-tx-hpd: hpd pin +- core-vdda-supply: phandle to supply regulator +- hdmi-mux-supply: phandle to mux regulator + +Optional properties: +- qcom,hdmi-tx-mux-en: hdmi mux enable pin +- qcom,hdmi-tx-mux-sel: hdmi mux select pin + +Example: + +/ { + ... + + hdmi: qcom,hdmi-tx-8960@4a00000 { + compatible = "qcom,hdmi-tx-8960"; + reg-names = "core_physical"; + reg = <0x04a00000 0x1000>; + interrupts = <GIC_SPI 79 0>; + clock-names = + "core_clk", + "master_iface_clk", + "slave_iface_clk"; + clocks = + <&mmcc HDMI_APP_CLK>, + <&mmcc HDMI_M_AHB_CLK>, + <&mmcc HDMI_S_AHB_CLK>; + qcom,hdmi-tx-ddc-clk = <&msmgpio 70 GPIO_ACTIVE_HIGH>; + qcom,hdmi-tx-ddc-data = <&msmgpio 71 GPIO_ACTIVE_HIGH>; + qcom,hdmi-tx-hpd = <&msmgpio 72 GPIO_ACTIVE_HIGH>; + core-vdda-supply = <&pm8921_hdmi_mvs>; + hdmi-mux-supply = <&ext_3p3v>; + }; +}; diff --git a/Documentation/devicetree/bindings/drm/msm/msm.txt b/Documentation/devicetree/bindings/drm/msm/msm.txt new file mode 100644 index 0000000..484cc12 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/msm/msm.txt @@ -0,0 +1,40 @@ +Qualcomm adreno/snapdragon + +Required properties: +- compatible: "qcom,mdp" (mdp4) or "qcom,mdss_mdp" (mdp5) +- reg: Physical base address and length of the controller's registers. +- interrupts: The interrupt outputs from the controller. +- connectors: array of phandles for output device(s) +- clocks: device clocks + See ../clocks/clock-bindings.txt for details. + +Optional properties: +- gpus: phandle for gpu device + +Example: + +/ { + ... + + mdp: qcom,mdp@5100000 { + compatible = "qcom,mdp"; + reg = <0x05100000 0xf0000>; + interrupts = <GIC_SPI 75 0>; + connectors = <&hdmi>; + gpus = <&gpu>; + clock-names = + "core_clk", + "iface_clk", + "lut_clk", + "src_clk", + "hdmi_clk", + "mdp_clk"; + clocks = + <&mmcc MDP_SRC>, + <&mmcc MDP_AHB_CLK>, + <&mmcc MDP_LUT_CLK>, + <&mmcc TV_SRC>, + <&mmcc HDMI_TV_CLK>, + <&mmcc MDP_TV_CLK>; + }; +}; diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c index a2cee06..2773600 100644 --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c @@ -680,6 +680,8 @@ static int a3xx_remove(struct platform_device *pdev) } static const struct of_device_id dt_match[] = { + { .compatible = "qcom,adreno-3xx" }, + /* for backwards compat w/ downstream kgsl DT files: */ { .compatible = "qcom,kgsl-3d0" }, {} }; diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 7f7aade..0ff8d46 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -123,7 +123,8 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder) for (i = 0; i < config->hpd_reg_cnt; i++) { struct regulator *reg; - reg = devm_regulator_get(&pdev->dev, config->hpd_reg_names[i]); + reg = devm_regulator_get_exclusive(&pdev->dev, + config->hpd_reg_names[i]); if (IS_ERR(reg)) { ret = PTR_ERR(reg); dev_err(dev->dev, "failed to get hpd regulator: %s (%d)\n", @@ -138,7 +139,8 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder) for (i = 0; i < config->pwr_reg_cnt; i++) { struct regulator *reg; - reg = devm_regulator_get(&pdev->dev, config->pwr_reg_names[i]); + reg = devm_regulator_get_exclusive(&pdev->dev, + config->pwr_reg_names[i]); if (IS_ERR(reg)) { ret = PTR_ERR(reg); dev_err(dev->dev, "failed to get pwr regulator: %s (%d)\n", @@ -273,24 +275,37 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data) return gpio; } - /* TODO actually use DT.. */ - static const char *hpd_reg_names[] = {"hpd-gdsc", "hpd-5v"}; - static const char *pwr_reg_names[] = {"core-vdda", "core-vcc"}; - static const char *hpd_clk_names[] = {"iface_clk", "core_clk", "mdp_core_clk"}; - static unsigned long hpd_clk_freq[] = {0, 19200000, 0}; - static const char *pwr_clk_names[] = {"extp_clk", "alt_iface_clk"}; + if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8x74")) { + static const char *hpd_reg_names[] = {"hpd-gdsc", "hpd-5v"}; + static const char *pwr_reg_names[] = {"core-vdda", "core-vcc"}; + static const char *hpd_clk_names[] = {"iface_clk", "core_clk", "mdp_core_clk"}; + static unsigned long hpd_clk_freq[] = {0, 19200000, 0}; + static const char *pwr_clk_names[] = {"extp_clk", "alt_iface_clk"}; + config.phy_init = hdmi_phy_8x74_init; + config.hpd_reg_names = hpd_reg_names; + config.hpd_reg_cnt = ARRAY_SIZE(hpd_reg_names); + config.pwr_reg_names = pwr_reg_names; + config.pwr_reg_cnt = ARRAY_SIZE(pwr_reg_names); + config.hpd_clk_names = hpd_clk_names; + config.hpd_freq = hpd_clk_freq; + config.hpd_clk_cnt = ARRAY_SIZE(hpd_clk_names); + config.pwr_clk_names = pwr_clk_names; + config.pwr_clk_cnt = ARRAY_SIZE(pwr_clk_names); + } else if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8960")) { + static const char *hpd_clk_names[] = {"core_clk", "master_iface_clk", "slave_iface_clk"}; + static const char *hpd_reg_names[] = {"core-vdda", "hdmi-mux"}; + config.phy_init = hdmi_phy_8960_init; + config.hpd_reg_names = hpd_reg_names; + config.hpd_reg_cnt = ARRAY_SIZE(hpd_reg_names); + config.hpd_clk_names = hpd_clk_names; + config.hpd_clk_cnt = ARRAY_SIZE(hpd_clk_names); + } else if (of_device_is_compatible(of_node, "qcom,hdmi-tx-8x60")) { + config.phy_init = hdmi_phy_8x60_init; + } else { + dev_err(dev, "unknown phy: %s\n", of_node->name); + } - config.phy_init = hdmi_phy_8x74_init; config.mmio_name = "core_physical"; - config.hpd_reg_names = hpd_reg_names; - config.hpd_reg_cnt = ARRAY_SIZE(hpd_reg_names); - config.pwr_reg_names = pwr_reg_names; - config.pwr_reg_cnt = ARRAY_SIZE(pwr_reg_names); - config.hpd_clk_names = hpd_clk_names; - config.hpd_freq = hpd_clk_freq; - config.hpd_clk_cnt = ARRAY_SIZE(hpd_clk_names); - config.pwr_clk_names = pwr_clk_names; - config.pwr_clk_cnt = ARRAY_SIZE(pwr_clk_names); config.ddc_clk_gpio = get_gpio("qcom,hdmi-tx-ddc-clk"); config.ddc_data_gpio = get_gpio("qcom,hdmi-tx-ddc-data"); config.hpd_gpio = get_gpio("qcom,hdmi-tx-hpd"); @@ -373,7 +388,9 @@ static int hdmi_dev_remove(struct platform_device *pdev) } static const struct of_device_id dt_match[] = { - { .compatible = "qcom,hdmi-tx" }, + { .compatible = "qcom,hdmi-tx-8x74" }, + { .compatible = "qcom,hdmi-tx-8960" }, + { .compatible = "qcom,hdmi-tx-8x60" }, {} }; diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c index 0bb4faa..5a7bfd4 100644 --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c @@ -294,15 +294,17 @@ struct msm_kms *mdp4_kms_init(struct drm_device *dev) goto fail; } - mdp4_kms->dsi_pll_vdda = devm_regulator_get(&pdev->dev, "dsi_pll_vdda"); + mdp4_kms->dsi_pll_vdda = + devm_regulator_get_optional(&pdev->dev, "dsi_pll_vdda"); if (IS_ERR(mdp4_kms->dsi_pll_vdda)) mdp4_kms->dsi_pll_vdda = NULL; - mdp4_kms->dsi_pll_vddio = devm_regulator_get(&pdev->dev, "dsi_pll_vddio"); + mdp4_kms->dsi_pll_vddio = + devm_regulator_get_optional(&pdev->dev, "dsi_pll_vddio"); if (IS_ERR(mdp4_kms->dsi_pll_vddio)) mdp4_kms->dsi_pll_vddio = NULL; - mdp4_kms->vdd = devm_regulator_get(&pdev->dev, "vdd"); + mdp4_kms->vdd = devm_regulator_get_exclusive(&pdev->dev, "vdd"); if (IS_ERR(mdp4_kms->vdd)) mdp4_kms->vdd = NULL; @@ -406,6 +408,8 @@ static struct mdp4_platform_config *mdp4_get_config(struct platform_device *dev) static struct mdp4_platform_config config = {}; #ifdef CONFIG_OF /* TODO */ + config.max_clk = 266667000; + config.iommu = iommu_domain_alloc(&platform_bus_type); #else if (cpu_is_apq8064()) config.max_clk = 266667000; diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 9a5d87d..1ad0155 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -906,7 +906,8 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == data; } -static int msm_drm_add_components(struct device *master, struct master *m) +static int add_components(struct device *master, struct master *m, + const char *name) { struct device_node *np = master->of_node; unsigned i; @@ -915,16 +916,35 @@ static int msm_drm_add_components(struct device *master, struct master *m) for (i = 0; ; i++) { struct device_node *node; - node = of_parse_phandle(np, "connectors", i); + node = of_parse_phandle(np, name, i); if (!node) break; ret = component_master_add_child(m, compare_of, node); of_node_put(node); - if (ret) + if (ret) { + dev_err(master, "could not add %s child %s: %d\n", + name, node->name, ret); return ret; + } } + + return 0; +} + +static int msm_drm_add_components(struct device *master, struct master *m) +{ + int ret; + + ret = add_components(master, m, "connectors"); + if (ret) + return ret; + + ret = add_components(master, m, "gpus"); + if (ret) + return ret; + return 0; } #else @@ -1008,7 +1028,8 @@ static const struct platform_device_id msm_id[] = { }; static const struct of_device_id dt_match[] = { - { .compatible = "qcom,mdss_mdp" }, + { .compatible = "qcom,mdp" }, /* mdp4 */ + { .compatible = "qcom,mdss_mdp" }, /* mdp5 */ {} }; MODULE_DEVICE_TABLE(of, dt_match);
Now that we (almost) have enough dependencies in place (MMCC, RPM, etc), add necessary DT support so that we can use drm/msm on upstream kernel. Signed-off-by: Rob Clark <robdclark@gmail.com> --- Commence bikeshedding :-) Documentation/devicetree/bindings/drm/msm/gpu.txt | 51 ++++++++++++++++++++ Documentation/devicetree/bindings/drm/msm/hdmi.txt | 43 +++++++++++++++++ Documentation/devicetree/bindings/drm/msm/msm.txt | 40 ++++++++++++++++ drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 2 + drivers/gpu/drm/msm/hdmi/hdmi.c | 55 ++++++++++++++-------- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 10 ++-- drivers/gpu/drm/msm/msm_drv.c | 29 ++++++++++-- 7 files changed, 204 insertions(+), 26 deletions(-) create mode 100644 Documentation/devicetree/bindings/drm/msm/gpu.txt create mode 100644 Documentation/devicetree/bindings/drm/msm/hdmi.txt create mode 100644 Documentation/devicetree/bindings/drm/msm/msm.txt