[U-Boot] [PATCH 2/4] video: tegra: Move to using simple-panel and pwm-backlight

[Simon Glass]

Hi Stephen,

On 19 January 2016 at 09:49, Stephen Warren <swarren at wwwdotorg.org> wrote:
>
> On 01/18/2016 07:08 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 18 January 2016 at 12:43, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>
>>> On 01/14/2016 01:26 PM, Simon Glass wrote:
>>>>
>>>>
>>>> We have standard drivers for panels and backlights which can do most of
>>>> the
>>>> work for us. Move the tegra20 LCD driver over to use those instead of
>>>> custom
>>>> code.
>>>>
>>>> This patch includes device tree changes for the nvidia boards. I have only
>>>> been able to test seaboard. If this patch is applied, these boards will
>>>> also need to be synced with the kernel, and updated to use
>>>> display-timings:
>>>>
>>>>      - colibri
>>>>      - medcom-wide
>>>>      - paz00
>>>>      - tec
>>>
>>>
>>>
>>>> diff --git a/arch/arm/dts/tegra20-harmony.dts
>>>> b/arch/arm/dts/tegra20-harmony.dts
>>>
>>>
>>>
>>> This file has huge changes unrelated to panel, video, and backlight. Perhaps
>>> patch 1/4 was meant to sync the DTs for Harmony and Ventana too rather than
>>> just Seaboard, leaving this commit to make roughly identical DT changes for
>>> all 3 boards?
>>
>>
>> Yes I think that would be better. I started on seaboard since that's
>> the only one I have. But then I thought I could do the other two since
>> you or Tom should be able to test it.
>>
>>>
>>>> diff --git a/arch/arm/dts/tegra20-seaboard.dts
>>>> b/arch/arm/dts/tegra20-seaboard.dts
>>>
>>>
>>>
>>>> +
>>>> +                               display-timings {
>>>> +                                       timing at 0 {
>>>> +                                               /* Seaboard has 1366x768
>>>> */
>>>> +                                               clock-frequency =
>>>> <70600000>;
>>>> +                                               hactive = <1366>;
>>>> +                                               vactive = <768>;
>>>> +                                               hback-porch = <58>;
>>>> +                                               hfront-porch = <58>;
>>>> +                                               hsync-len = <58>;
>>>> +                                               vback-porch = <4>;
>>>> +                                               vfront-porch = <4>;
>>>> +                                               vsync-len = <4>;
>>>> +                                               hsync-active = <1>;
>>>> +                                       };
>>>> +                               };
>>>
>>>
>>>
>>> It seems very strange to go to the effort of creating patch 1 to sync the
>>> U-Boot and Linux DT files (such that I assume they're 100% identical?) yet
>>> immediately have patch 2 diverge the two DTs again.
>>>
>>> This lends more weight to my argument that the U-Boot DTs should contain
>>> only nodes that U-Boot actively uses. I can see the argument for copying
>>> over the entire DT from Linux into U-Boot in order to keep them 100%
>>> identical, and in turn that means importing a bunch of DT content that
>>> U-Boot doesn't use. However, that argument completely breaks down if the DTs
>>> are just going to diverge still.
>>>
>>> At least the display-timings node is a standard binding, even if it isn't
>>> one that the Linux Tegra DTs make use of.
>>
>>
>> How about adding this to the Linux Tegra DT? It's a hardware
>> description so shouldn't ruffle too many feathres.
>>
>>>
>>>> diff --git a/configs/harmony_defconfig b/configs/harmony_defconfig
>>>> index 14125b4..0de56a9 100644
>>>> --- a/configs/harmony_defconfig
>>>> +++ b/configs/harmony_defconfig
>>>> @@ -12,9 +12,11 @@ CONFIG_SYS_PROMPT="Tegra20 (Harmony) # "
>>>>    CONFIG_CMD_GPIO=y
>>>>    # CONFIG_CMD_SETEXPR is not set
>>>>    # CONFIG_CMD_NFS is not set
>>>> +CONFIG_CMD_PMIC=y
>>>> +CONFIG_CMD_REGULATOR=y
>>>>    CONFIG_DM_PMIC=y
>>>>    CONFIG_DM_REGULATOR=y
>>>> -CONFIG_DM_PWM=y
>>>> +CONFIG_DM_REGULATOR_FIXED=y
>>>>    CONFIG_PWM_TEGRA=y
>>>>    CONFIG_SYS_NS16550=y
>>>>    CONFIG_USB=y
>>>> @@ -22,3 +24,4 @@ CONFIG_DM_USB=y
>>>>    CONFIG_DM_VIDEO=y
>>>>    CONFIG_VIDEO_TEGRA20=y
>>>>    CONFIG_USE_PRIVATE_LIBGCC=y
>>>> +CONFIG_ERRNO_STR=y
>>>
>>>
>>>
>>> I assume those are all needed by the modified Tegra video/LCD driver. If so,
>>> wouldn't it make sense for that driver to select those Kconfig options
>>> rather than requiring every board defconfig to be modified to enable the
>>> features?
>>
>>
>> Do you mean 'depends on' in the Kconfig? This is a discussion that
>> goes back and forth...I don't really mind. It would mean that you
>> would have to play detective in menuconfig to turn the display on for
>> a board: find all the options it depends on, turn them on one by one
>> and then the display option appears.
>
>
> I was thinking of "select" in Kconfig, so that if someone enables Tegra LCD/display support then regulators etc. get automatically enabled too.
>
> But "depends" would work too.

'select' sounds better. I'll take a look.

Regards,
Simon