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: 4916 bytes --]

On Mon, Dec 20, 2021 at 07:12:03PM +0300, Dmitry Osipenko wrote:
> 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@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.

Yes, it's completely relevant because it essentially replaces the I2C
DDC. With the AUX bus, the AUX and hence the I2C DDC can be implied from
the bus' parent.

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

All the (at least relevant) functionality that is in panel-edp was in
panel-simple before it was moved to panel-edp. I've backported this set
of patches to v5.15 and it works just fine there.

Thierry

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