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

[Stephen Warren <swarren@wwwdotorg.org>]

On 01/18/2016 07:08 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 18 January 2016 at 12:43, Stephen Warren <swarren@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.