Add DSS node for J721E SoC.
Signed-off-by: Tomi Valkeinen <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 57 +++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
index 0b9d14b838a1..21c362042ecf 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
@@ -736,6 +736,63 @@
};
};
+ dss: dss@04a00000 {
+ compatible = "ti,j721e-dss";
+ reg =
+ <0x00 0x04a00000 0x00 0x10000>, /* common_m */
+ <0x00 0x04a10000 0x00 0x10000>, /* common_s0*/
+ <0x00 0x04b00000 0x00 0x10000>, /* common_s1*/
+ <0x00 0x04b10000 0x00 0x10000>, /* common_s2*/
+
+ <0x00 0x04a20000 0x00 0x10000>, /* vidl1 */
+ <0x00 0x04a30000 0x00 0x10000>, /* vidl2 */
+ <0x00 0x04a50000 0x00 0x10000>, /* vid1 */
+ <0x00 0x04a60000 0x00 0x10000>, /* vid2 */
+
+ <0x00 0x04a70000 0x00 0x10000>, /* ovr1 */
+ <0x00 0x04a90000 0x00 0x10000>, /* ovr2 */
+ <0x00 0x04ab0000 0x00 0x10000>, /* ovr3 */
+ <0x00 0x04ad0000 0x00 0x10000>, /* ovr4 */
+
+ <0x00 0x04a80000 0x00 0x10000>, /* vp1 */
+ <0x00 0x04aa0000 0x00 0x10000>, /* vp2 */
+ <0x00 0x04ac0000 0x00 0x10000>, /* vp3 */
+ <0x00 0x04ae0000 0x00 0x10000>, /* vp4 */
+ <0x00 0x04af0000 0x00 0x10000>; /* wb */
+
+ reg-names = "common_m", "common_s0",
+ "common_s1", "common_s2",
+ "vidl1", "vidl2","vid1","vid2",
+ "ovr1", "ovr2", "ovr3", "ovr4",
+ "vp1", "vp2", "vp3", "vp4",
+ "wb";
+
+ clocks = <&k3_clks 152 0>,
+ <&k3_clks 152 1>,
+ <&k3_clks 152 4>,
+ <&k3_clks 152 9>,
+ <&k3_clks 152 13>;
+ clock-names = "fck", "vp1", "vp2", "vp3", "vp4";
+
+ power-domains = <&k3_pds 152 TI_SCI_PD_EXCLUSIVE>;
+
+ interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 603 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "common_m",
+ "common_s0",
+ "common_s1",
+ "common_s2";
+
+ status = "disabled";
+
+ dss_ports: ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
mcasp0: mcasp@2b00000 {
compatible = "ti,am33xx-mcasp-audio";
reg = <0x0 0x02b00000 0x0 0x2000>,
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 22/04/2020 12:15, Tomi Valkeinen wrote:
> Add DSS node for J721E SoC.
Subject should drop .dtsi, I can fix that locally though. Got a question
below.
>
> Signed-off-by: Tomi Valkeinen <[email protected]>
> ---
> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 57 +++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> index 0b9d14b838a1..21c362042ecf 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> @@ -736,6 +736,63 @@
> };
> };
>
> + dss: dss@04a00000 {
> + compatible = "ti,j721e-dss";
> + reg =
> + <0x00 0x04a00000 0x00 0x10000>, /* common_m */
> + <0x00 0x04a10000 0x00 0x10000>, /* common_s0*/
> + <0x00 0x04b00000 0x00 0x10000>, /* common_s1*/
> + <0x00 0x04b10000 0x00 0x10000>, /* common_s2*/
> +
> + <0x00 0x04a20000 0x00 0x10000>, /* vidl1 */
> + <0x00 0x04a30000 0x00 0x10000>, /* vidl2 */
> + <0x00 0x04a50000 0x00 0x10000>, /* vid1 */
> + <0x00 0x04a60000 0x00 0x10000>, /* vid2 */
> +
> + <0x00 0x04a70000 0x00 0x10000>, /* ovr1 */
> + <0x00 0x04a90000 0x00 0x10000>, /* ovr2 */
> + <0x00 0x04ab0000 0x00 0x10000>, /* ovr3 */
> + <0x00 0x04ad0000 0x00 0x10000>, /* ovr4 */
> +
> + <0x00 0x04a80000 0x00 0x10000>, /* vp1 */
> + <0x00 0x04aa0000 0x00 0x10000>, /* vp2 */
> + <0x00 0x04ac0000 0x00 0x10000>, /* vp3 */
> + <0x00 0x04ae0000 0x00 0x10000>, /* vp4 */
> + <0x00 0x04af0000 0x00 0x10000>; /* wb */
> +
> + reg-names = "common_m", "common_s0",
> + "common_s1", "common_s2",
> + "vidl1", "vidl2","vid1","vid2",
> + "ovr1", "ovr2", "ovr3", "ovr4",
> + "vp1", "vp2", "vp3", "vp4",
> + "wb";
> +
> + clocks = <&k3_clks 152 0>,
> + <&k3_clks 152 1>,
> + <&k3_clks 152 4>,
> + <&k3_clks 152 9>,
> + <&k3_clks 152 13>;
> + clock-names = "fck", "vp1", "vp2", "vp3", "vp4";
> +
> + power-domains = <&k3_pds 152 TI_SCI_PD_EXCLUSIVE>;
> +
> + interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 603 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "common_m",
> + "common_s0",
> + "common_s1",
> + "common_s2";
> +
> + status = "disabled";
Again, why disabled by default?
-Tero
> +
> + dss_ports: ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +
> mcasp0: mcasp@2b00000 {
> compatible = "ti,am33xx-mcasp-audio";
> reg = <0x0 0x02b00000 0x0 0x2000>,
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 27/04/2020 13:09, Tero Kristo wrote:
>> + status = "disabled";
>
> Again, why disabled by default?
>
tidss device is not functional without a defined video-port. The driver
is not implemented in a way that it would handle a broken configuration
gracefully.
Best regards,
Jyri
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 27/04/2020 13:37, Jyri Sarha wrote:
> On 27/04/2020 13:09, Tero Kristo wrote:
>>> + status = "disabled";
>>
>> Again, why disabled by default?
>>
>
> tidss device is not functional without a defined video-port. The driver
> is not implemented in a way that it would handle a broken configuration
> gracefully.
What/where/when is the video-port going to be defined then? Is this
going to be done in an overlay?
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 27/04/2020 13:41, Tero Kristo wrote:
> On 27/04/2020 13:37, Jyri Sarha wrote:
>> On 27/04/2020 13:09, Tero Kristo wrote:
>>>> + status = "disabled";
>>>
>>> Again, why disabled by default?
>>>
>>
>> tidss device is not functional without a defined video-port. The driver
>> is not implemented in a way that it would handle a broken configuration
>> gracefully.
>
> What/where/when is the video-port going to be defined then? Is this
> going to be done in an overlay?
>
Yes. It should be defined in the board specific dts or dtso file, where
the video-connector or -panel is.
BR,
Jyri
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 27/04/2020 13:37, Jyri Sarha wrote:
> On 27/04/2020 13:09, Tero Kristo wrote:
>>> + status = "disabled";
>>
>> Again, why disabled by default?
>>
>
> tidss device is not functional without a defined video-port. The driver
> is not implemented in a way that it would handle a broken configuration
> gracefully.
Then we need to fix it. The driver should handle the case where there are no ports defined just fine.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 27/04/2020 13:51, Tomi Valkeinen wrote:
> On 27/04/2020 13:37, Jyri Sarha wrote:
>> On 27/04/2020 13:09, Tero Kristo wrote:
>>>> + status = "disabled";
>>>
>>> Again, why disabled by default?
>>>
>>
>> tidss device is not functional without a defined video-port. The driver
>> is not implemented in a way that it would handle a broken configuration
>> gracefully.
>
> Then we need to fix it. The driver should handle the case where there
> are no ports defined just fine.
>
Just by reading the code, I would say that currently the probe would
fail with returned -ENOMEM after calling drm_vblank_init() with zero CRTCs.
So should the probe fail gracefully and silently, or should we try to
register a DRM device with no CRTCs? Is that even possible?
BR,
Jyri
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 27/04/2020 14:10, Jyri Sarha wrote:
> On 27/04/2020 13:51, Tomi Valkeinen wrote:
>> On 27/04/2020 13:37, Jyri Sarha wrote:
>>> On 27/04/2020 13:09, Tero Kristo wrote:
>>>>> + status = "disabled";
>>>>
>>>> Again, why disabled by default?
>>>>
>>>
>>> tidss device is not functional without a defined video-port. The driver
>>> is not implemented in a way that it would handle a broken configuration
>>> gracefully.
>>
>> Then we need to fix it. The driver should handle the case where there
>> are no ports defined just fine.
>>
>
> Just by reading the code, I would say that currently the probe would
> fail with returned -ENOMEM after calling drm_vblank_init() with zero CRTCs.
>
> So should the probe fail gracefully and silently, or should we try to
> register a DRM device with no CRTCs? Is that even possible?
My first thought is that the driver should exit probe silently with ENODEV if there are no outputs
defined (but, of course, with EPROBE_DEFER if there are outputs which haven't been probed yet).
It gets a bit more complex if we ever support writeback, as that can be used as mem-to-mem without
any displays, but I think we can ignore that for now.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 27/04/2020 14:15, Tomi Valkeinen wrote:
> On 27/04/2020 14:10, Jyri Sarha wrote:
>> On 27/04/2020 13:51, Tomi Valkeinen wrote:
>>> On 27/04/2020 13:37, Jyri Sarha wrote:
>>>> On 27/04/2020 13:09, Tero Kristo wrote:
>>>>>> + status = "disabled";
>>>>>
>>>>> Again, why disabled by default?
>>>>>
>>>>
>>>> tidss device is not functional without a defined video-port. The driver
>>>> is not implemented in a way that it would handle a broken configuration
>>>> gracefully.
>>>
>>> Then we need to fix it. The driver should handle the case where there
>>> are no ports defined just fine.
>>>
>>
>> Just by reading the code, I would say that currently the probe would
>> fail with returned -ENOMEM after calling drm_vblank_init() with zero CRTCs.
>>
>> So should the probe fail gracefully and silently, or should we try to
>> register a DRM device with no CRTCs? Is that even possible?
>
> My first thought is that the driver should exit probe silently with ENODEV if there are no outputs
> defined (but, of course, with EPROBE_DEFER if there are outputs which haven't been probed yet).
>
> It gets a bit more complex if we ever support writeback, as that can be used as mem-to-mem without
> any displays, but I think we can ignore that for now.
In any case, that's not the reason for status = "disabled", so that discussion is not related to
these patches as such.
The reason to have DSS disabled is just to prevent pointless driver probing. When a board dts or a
DT overlay adds a display, the DSS DT node has to be modified anyway to add the DT graph and the
panel/bridge data. So one can as well add the single line of "status = enabled" there.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 27/04/2020 14:37, Tomi Valkeinen wrote:
> On 27/04/2020 14:15, Tomi Valkeinen wrote:
>> On 27/04/2020 14:10, Jyri Sarha wrote:
>>> On 27/04/2020 13:51, Tomi Valkeinen wrote:
>>>> On 27/04/2020 13:37, Jyri Sarha wrote:
>>>>> On 27/04/2020 13:09, Tero Kristo wrote:
>>>>>>> + status = "disabled";
>>>>>>
>>>>>> Again, why disabled by default?
>>>>>>
>>>>>
>>>>> tidss device is not functional without a defined video-port. The
>>>>> driver
>>>>> is not implemented in a way that it would handle a broken
>>>>> configuration
>>>>> gracefully.
>>>>
>>>> Then we need to fix it. The driver should handle the case where there
>>>> are no ports defined just fine.
>>>>
>>>
>>> Just by reading the code, I would say that currently the probe would
>>> fail with returned -ENOMEM after calling drm_vblank_init() with zero
>>> CRTCs.
>>>
>>> So should the probe fail gracefully and silently, or should we try to
>>> register a DRM device with no CRTCs? Is that even possible?
>>
>> My first thought is that the driver should exit probe silently with
>> ENODEV if there are no outputs defined (but, of course, with
>> EPROBE_DEFER if there are outputs which haven't been probed yet).
>>
>> It gets a bit more complex if we ever support writeback, as that can
>> be used as mem-to-mem without any displays, but I think we can ignore
>> that for now.
>
> In any case, that's not the reason for status = "disabled", so that
> discussion is not related to these patches as such.
>
> The reason to have DSS disabled is just to prevent pointless driver
> probing. When a board dts or a DT overlay adds a display, the DSS DT
> node has to be modified anyway to add the DT graph and the panel/bridge
> data. So one can as well add the single line of "status = enabled" there.
Ok, thanks for the explanation, queued all three patches towards 5.8
based on that.
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki