[PATCH 0/2] drm/tegra: Fix panel support on Venice 2 and Nyan

[Dmitry Osipenko]

20.12.2021 18:27, Thierry Reding пишет:
> On Mon, Dec 20, 2021 at 05:45:41PM +0300, Dmitry Osipenko wrote:
>> 20.12.2021 13:48, Thierry Reding пишет:
>>> From: Thierry Reding <treding at nvidia.com>
>>>
>>> Hi,
>>>
>>> this is an alternative proposal to fix panel support on Venice 2 and
>>> Nyan. Dmitry had proposed a different solution that involved reverting
>>> the I2C/DDC registration order and would complicate things by breaking
>>> the encapsulation of the driver by introducing a global (though locally
>>> scoped) variable[0].
>>>
>>> This set of patches avoids that by using the recently introduced DP AUX
>>> bus infrastructure. The result is that the changes are actually less
>>> intrusive and not a step back. Instead they nicely remove the circular
>>> dependency that previously existed and caused these issues in the first
>>> place.
>>>
>>> To be fair, this is not perfect either because it requires a device tree
>>> change and hence isn't technically backwards-compatible. However, given
>>> that the original device tree was badly broken in the first place, I
>>> think we can make an exception, especially since it is not generally a
>>> problem to update device trees on the affected devices.
>>>
>>> Secondly, this relies on infrastructure that was introduced in v5.15 and
>>> therefore will be difficult to backport beyond that. However, since this
>>> functionality has been broken since v5.13 and all of the kernel versions
>>> between that and v5.15 are EOL anyway, there isn't much that we can do
>>> to fix the interim versions anyway.
>>>
>>> Adding Doug and Laurent since they originally designed the AUX bus
>>> patches in case they see anything in here that would be objectionable.
>>>
>>> Thierry
>>>
>>> [0]: https://lore.kernel.org/dri-devel/20211130230957.30213-1-digetx@gmail.com/
>>>
>>> Thierry Reding (2):
>>>   drm/tegra: dpaux: Populate AUX bus
>>>   ARM: tegra: Move panels to AUX bus
>>>
>>>  arch/arm/boot/dts/tegra124-nyan-big.dts   | 15 +++++++++------
>>>  arch/arm/boot/dts/tegra124-nyan-blaze.dts | 15 +++++++++------
>>>  arch/arm/boot/dts/tegra124-venice2.dts    | 14 +++++++-------
>>>  drivers/gpu/drm/tegra/Kconfig             |  1 +
>>>  drivers/gpu/drm/tegra/dpaux.c             |  7 +++++++
>>>  5 files changed, 33 insertions(+), 19 deletions(-)
>>>
>>
>> It should "work" since you removed the ddc-i2c-bus phandle from the
>> panel nodes, and thus, panel->ddc won't be used during panel-edp driver
>> probe. But this looks like a hack rather than a fix.
> 
> The AUX ->ddc will be used for panel->ddc if the ddc-i2c-bus property is
> not specified. And that makes perfect sense because we'd basically just
> be pointing back to the AUX node anyway.
> 
>> I'm not sure why and how devm_of_dp_aux_populate_ep_devices() usage
>> should be relevant here. The drm_dp_aux_register() still should to
>> invoked before devm_of_dp_aux_populate_ep_devices(), otherwise
>> panel->ddc adapter won't be registered.
> 
> drm_dp_aux_register() is only needed to expose the device to userspace
> and make the I2C adapter available to the rest of the system. But since
> we already know the AUX and I2C adapter, we can use it directly without
> doing a separate lookup. drm_dp_aux_init() should be enough to set the
> adapter up to work for what we need.
> 
> See also the kerneldoc for drm_dp_aux_register() where this is described
> in a bit more detail.

Alright, so you fixed it by removing the ddc-i2c-bus phandles and I2C
DDC will work properly anyways with that on v5.16.

But the aux-bus usage still is irrelevant for the fix. Let's not use it
then.

>> The panel->ddc isn't used by the new panel-edp driver unless panel is
>> compatible with "edp-panel". Hence the generic_edp_panel_probe() should
>> either fail or crash for a such "edp-panel" since panel->ddc isn't fully
>> instantiated, AFAICS.
> 
> I've tested this and it works fine on Venice 2. Since that was the
> reference design for Nyan, I suspect that Nyan's will also work.
> 
> It'd be great if Thomas or anyone else with access to a Nyan could
> test this to verify that.

There is no panel-edp driver in the v5.15. The EOL of v5.15 is Oct,
2023, hence we need to either use:

Approach #1:

1. apply my variant of the fix
2. backport it to v5.15
3. apply your variant without aux-bus, replacing my fix on 5.16+

Or

Approach #2:

1. remove ddc-i2c-bus phandles in DTs
2. backport (?) it to v5.15
3. apply your variant without aux-bus

Or

Approach #3:

1. ignore v5.15 and keep it screwed
2. apply your variant as is

Which approach will you prefer?

I'm not happy that older DTBs will be broken. Can we do better about it?

Is it possible to patch DT in the code by removing the ddc-i2c-bus phandle?

Or can we patch panel-simple on 5.15 and panel-edp on 5.16, making these
drivers to skip ddc-i2c-bus on Tegra+eDP? The eDP driver fixup will be
trivial, fixing older panel-simple will be more invasive.

I think the best solution will be to use Approach #1 and in the end
complete it with this panel-edp workaround below. This approach will
have minimal code impact on 5.16+ kernels and will keep older DTBs
working. Are you okay with this?

diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 176ef0c3cc1d..4e5b84324659 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -793,7 +793,11 @@ static int panel_edp_probe(struct device *dev,
const struct panel_desc *desc,
 		return err;
 	}

-	ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0);
+	if (of_machine_is_compatible("nvidia,tegra124"))
+		ddc = NULL;
+	else
+		ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0);
+
 	if (ddc) {
 		panel->ddc = of_find_i2c_adapter_by_node(ddc);
 		of_node_put(ddc);