20.12.2021 19:12, 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@xxxxxxxxxx> >>>> >>>> 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@xxxxxxxxx/ >>>> >>>> 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+ Although, I see that it doesn't make much sense to say "your variant without aux-bus". "Remove ddc-i2c-bus phandles from DTs" will be better. > 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); > Another alternative that may work is to check whether ddc-i2c-bus and DPAUX use the same node. diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 176ef0c3cc1d..c8cf5bc3d148 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -794,7 +794,7 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc, } ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0); - if (ddc) { + if (ddc && ddc != aux->dev->of_node) { panel->ddc = of_find_i2c_adapter_by_node(ddc); of_node_put(ddc);
