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

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3905 bytes --]

On Mon, Dec 20, 2021 at 05:45:41PM +0300, Dmitry Osipenko wrote:
> 20.12.2021 13:48, Thierry Reding пишет:
> > From: Thierry Reding <treding@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.

> 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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]