Re: [PATCH 1/2] drm/nouveau/bar/gf100: fix hang when calling ->fini() before ->init()
On 04/12/17 18:37, Guillaume Tucker wrote: > If the firmware fails to load then ->fini() will be called before the > device has been initialised, causing the kernel to hang while trying > to write to a register. Add a test in ->fini() to avoid this issue. > > This fixes a kernel hang on tegra124. > > Fixes: b17de35a2ebbe ("drm/nouveau/bar: implement bar1 teardown") > Signed-off-by: Guillaume Tucker <guillaume.tuc...@collabora.com> > CC: Ben Skeggs <bske...@redhat.com> > --- > drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c > index a3ba7f50198b..95e2aba64aad 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c > @@ -43,9 +43,12 @@ gf100_bar_bar1_wait(struct nvkm_bar *base) > } > > void > -gf100_bar_bar1_fini(struct nvkm_bar *bar) > +gf100_bar_bar1_fini(struct nvkm_bar *base) > { > - nvkm_mask(bar->subdev.device, 0x001704, 0x8000, 0x); > + struct nvkm_device *device = base->subdev.device; > + > + if (base->subdev.oneinit) > + nvkm_mask(device, 0x001704, 0x8000, 0x); > } > > void I have tested this and it works for me. Thanks for fixing this! Would be good to get Ben's ACK, but you can have my ... Tested-by: Jon Hunter <jonath...@nvidia.com> Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/nouveau/bar/gf100: fix hang when calling ->fini() before ->init()
On 06/12/17 17:18, Jon Hunter wrote: > > On 06/12/17 09:22, Guillaume Tucker wrote: >> On 05/12/17 18:32, Ben Skeggs wrote: >>> On Wed, Dec 6, 2017 at 12:30 AM, Jon Hunter <jonath...@nvidia.com> wrote: >>> >>>> >>>> On 04/12/17 18:37, Guillaume Tucker wrote: >>>>> If the firmware fails to load then ->fini() will be called before the >>>>> device has been initialised, causing the kernel to hang while trying >>>>> to write to a register. Add a test in ->fini() to avoid this issue. >>>>> >>>>> This fixes a kernel hang on tegra124. >>>>> >>>>> Fixes: b17de35a2ebbe ("drm/nouveau/bar: implement bar1 teardown") >>>>> Signed-off-by: Guillaume Tucker <guillaume.tuc...@collabora.com> >>>>> CC: Ben Skeggs <bske...@redhat.com> >>>>> --- >>>>> drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c | 7 +-- >>>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c >>>> b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c >>>>> index a3ba7f50198b..95e2aba64aad 100644 >>>>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c >>>>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c >>>>> @@ -43,9 +43,12 @@ gf100_bar_bar1_wait(struct nvkm_bar *base) >>>>> } >>>>> >>>>> void >>>>> -gf100_bar_bar1_fini(struct nvkm_bar *bar) >>>>> +gf100_bar_bar1_fini(struct nvkm_bar *base) >>>>> { >>>>> - nvkm_mask(bar->subdev.device, 0x001704, 0x8000, 0x); >>>>> + struct nvkm_device *device = base->subdev.device; >>>>> + >>>>> + if (base->subdev.oneinit) >>>>> + nvkm_mask(device, 0x001704, 0x8000, 0x); >>>>> } >>>>> >>>>> void >>>> >>>> I have tested this and it works for me. Thanks for fixing this! Would be >>>> good to get Ben's ACK, but you can have my ... >>>> >>> I'd love to get a good explanation as to why it hangs without this >>> change, >>> as, on the surface, it's not immediately obvious as to why it's hanging. >> >> To be fair I'm not entirely sure either why this causes a hang, I >> haven't read the TRM... The iomem has been mapped at this point, >> so accessing the register should work. One clue is when you look >> at _bar1_init(), the 0x1704 register is initialised with >> some (device instance?) memory address. So it's possible that >> the hardware does something special when you set this to 0 as in >> _bar1_fini(), which may fail in particular if it was previously >> not initialised with a valid address. >> >> This is merely guesswork, would be interested to find out the >> real explanation though. > > OK, well that's no good. It's a good pointer, but we need to make sure > we understand the root of this hang. I will see if I have sometime to > dig into this further, maybe next week. I spent a bit of time looking at this, but I still do not fully understand the cause of the hang. It appears to hang initialisation of the FB subdev and it appears to be around the point where the L2 cache is flushed. I will see if anyone else has a clue what is happening here. Ben, in the meantime with the holiday season upon us, should we remove the bar1 teardown for gk20a? diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/base.c index 9646adec57cb..243f0a5c8a62 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/base.c @@ -73,7 +73,8 @@ struct nvkm_vmm * nvkm_bar_fini(struct nvkm_subdev *subdev, bool suspend) { struct nvkm_bar *bar = nvkm_bar(subdev); - bar->func->bar1.fini(bar); + if (bar->func->bar1.fini) + bar->func->bar1.fini(bar); return 0; } diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gk20a.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gk20a.c index b10077d38839..35878fb538f2 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gk20a.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gk20a.c @@ -26,7 +26,6 @@ .dtor = gf100_bar_dtor, .oneinit = gf100_bar_oneinit, .bar1.init = gf100_bar_bar1_init, - .bar1.fini = gf100_bar_bar1_fini, .bar1.wait = gf100_bar_bar1_wait, .bar1.vmm = gf100_bar_bar1_vmm, .flush = g84_bar_flush, Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/nouveau/bar/gf100: fix hang when calling ->fini() before ->init()
On 06/12/17 09:22, Guillaume Tucker wrote: > On 05/12/17 18:32, Ben Skeggs wrote: >> On Wed, Dec 6, 2017 at 12:30 AM, Jon Hunter <jonath...@nvidia.com> wrote: >> >>> >>> On 04/12/17 18:37, Guillaume Tucker wrote: >>>> If the firmware fails to load then ->fini() will be called before the >>>> device has been initialised, causing the kernel to hang while trying >>>> to write to a register. Add a test in ->fini() to avoid this issue. >>>> >>>> This fixes a kernel hang on tegra124. >>>> >>>> Fixes: b17de35a2ebbe ("drm/nouveau/bar: implement bar1 teardown") >>>> Signed-off-by: Guillaume Tucker <guillaume.tuc...@collabora.com> >>>> CC: Ben Skeggs <bske...@redhat.com> >>>> --- >>>> drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c | 7 +-- >>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c >>> b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c >>>> index a3ba7f50198b..95e2aba64aad 100644 >>>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c >>>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gf100.c >>>> @@ -43,9 +43,12 @@ gf100_bar_bar1_wait(struct nvkm_bar *base) >>>> } >>>> >>>> void >>>> -gf100_bar_bar1_fini(struct nvkm_bar *bar) >>>> +gf100_bar_bar1_fini(struct nvkm_bar *base) >>>> { >>>> - nvkm_mask(bar->subdev.device, 0x001704, 0x8000, 0x); >>>> + struct nvkm_device *device = base->subdev.device; >>>> + >>>> + if (base->subdev.oneinit) >>>> + nvkm_mask(device, 0x001704, 0x8000, 0x); >>>> } >>>> >>>> void >>> >>> I have tested this and it works for me. Thanks for fixing this! Would be >>> good to get Ben's ACK, but you can have my ... >>> >> I'd love to get a good explanation as to why it hangs without this >> change, >> as, on the surface, it's not immediately obvious as to why it's hanging. > > To be fair I'm not entirely sure either why this causes a hang, I > haven't read the TRM... The iomem has been mapped at this point, > so accessing the register should work. One clue is when you look > at _bar1_init(), the 0x1704 register is initialised with > some (device instance?) memory address. So it's possible that > the hardware does something special when you set this to 0 as in > _bar1_fini(), which may fail in particular if it was previously > not initialised with a valid address. > > This is merely guesswork, would be interested to find out the > real explanation though. OK, well that's no good. It's a good pointer, but we need to make sure we understand the root of this hang. I will see if I have sometime to dig into this further, maybe next week. Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH v2 1/1] drm/tegra: sor: Fix hang on tegra124 due to NULL clk_out
On 20/12/17 18:15, Thierry Reding wrote: > On Wed, Dec 20, 2017 at 11:32:23AM +, Guillaume Tucker wrote: >> When neither HDMI nor DP is supported such as on the tegra124, the >> sor->clk_out is not initialised and remains NULL. In this case, the >> parent clock can't be assigned to it so revert to the previous >> behaviour of assigning it to the main sor->clk instead. >> >> This fixes a kernel hang on tegra124 and should also affect tegra210 >> as they both don't support HDMI and DP. Tested on tegra124 only. >> >> Fixes: e1335e2f0cfc ("drm/tegra: sor: Reimplement pad clock") >> Signed-off-by: Guillaume Tucker <guillaume.tuc...@collabora.com> >> CC: Thierry Reding <tred...@nvidia.com> >> --- >> drivers/gpu/drm/tegra/sor.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > How about just the below instead? It's one more line than your patch, > but it will automatically handle all occurrences of clk_out properly. > > --- >8 --- > diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c > index f6313c4d612e..4be9edf9c6fe 100644 > --- a/drivers/gpu/drm/tegra/sor.c > +++ b/drivers/gpu/drm/tegra/sor.c > @@ -3047,6 +3047,8 @@ static int tegra_sor_probe(struct platform_device *pdev) > name, err); > goto remove; > } > + } else { > + sor->clk_out = sor->clk; > } > > sor->clk_parent = devm_clk_get(>dev, "parent"); > --- >8 --- > > That said, I suspect the SOR might be compatible from a clock point of > view with later versions and perhaps we just didn't implement clocks > correctly back in the Tegra124 timeframe. > > Maybe Peter knows. So the above change from Thierry works for me and we need this for v4.15 (otherwise nyan-big does not boot) so you can have my ... Tested-by: Jon Hunter <jonath...@nvidia.com> However, would be good to have Peter's ACK, especially seeing that Tegra210 sor0 does not support HDMI and DP. So we need to make sure this is correct for Tegra210 as well (although I have not seen any regressions for Tegra210). Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/nouveau/bar/gk20a: Avoid bar teardown during init
Commit bbb163e18960 ("drm/nouveau/bar: implement bar1 teardown") introduced add a teardown helper function for BAR1. During initialisation of the Nouveau, initially all the teardown helpers are called once, before calling their init counterparts. For gk20a, after the BAR1 teardown function is called, the device is hanging during the initialisation of the FB sub-device. At this point it is unclear why this is happening and this is still under investigation. However, this change is preventing Tegra124 devices from booting when Nouveau is enabled. To allow Tegra124 to boot, remove the teardown helper for gk20a. This is based upon a previous patch by Guillaume Tucker but limits the workaround to only gk20a GPUs. Fixes: bbb163e18960 ("drm/nouveau/bar: implement bar1 teardown") Reported-by: Guillaume Tucker <guillaume.tuc...@collabora.com> Signed-off-by: Jon Hunter <jonath...@nvidia.com> --- I am not happy that we do not yet fully understand the cause of the hang, but I am talking with a few people at NVIDIA about this and have a few things to look into. However, given that we are close to v4.15 being released and I am not sure we will have a proper fix in place before, I think it is best to workaround this for now. drivers/gpu/drm/nouveau/nvkm/subdev/bar/base.c | 3 ++- drivers/gpu/drm/nouveau/nvkm/subdev/bar/gk20a.c | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/base.c index 9646adec57cb..243f0a5c8a62 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/base.c @@ -73,7 +73,8 @@ static int nvkm_bar_fini(struct nvkm_subdev *subdev, bool suspend) { struct nvkm_bar *bar = nvkm_bar(subdev); - bar->func->bar1.fini(bar); + if (bar->func->bar1.fini) + bar->func->bar1.fini(bar); return 0; } diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gk20a.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gk20a.c index b10077d38839..35878fb538f2 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gk20a.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bar/gk20a.c @@ -26,7 +26,6 @@ gk20a_bar_func = { .dtor = gf100_bar_dtor, .oneinit = gf100_bar_oneinit, .bar1.init = gf100_bar_bar1_init, - .bar1.fini = gf100_bar_bar1_fini, .bar1.wait = gf100_bar_bar1_wait, .bar1.vmm = gf100_bar_bar1_vmm, .flush = g84_bar_flush, -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] soc/tegra: pmc: set IO pad power state and voltage via pinctrl fw
On 02/08/18 12:59, Venkat Reddy Talla wrote: > The IO pins of Tegra SoCs are grouped for common control > of IO interface like setting voltage signal levels and > power state of the interface. These groups are referred > to as IO pads.The power state and voltage control of IO pins > can be done at IO pads level. > > Tegra SoCs support powering down IO pads when they are > not used even in the active state of system. > This saves power from that IO interface. Also it supports > multiple voltage level in IO pins for interfacing on > some of pads. The IO pad voltage is automatically detected > till Tegra124, hence SW need not to configure this. > But from Tegra210, the automatic detection logic has been > removed, hence SW need to explicitly set the IO pad > voltage into IO pad configuration registers. > > Add support to configure the power state and voltage level > of the IO pads from client driver via pincontrol framework. > > Signed-off-by: Venkat Reddy Talla This appears to be a duplicate effort of the following which we have been reviewing ... https://marc.info/?l=linux-tegra=153295930808915=2 Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [v3] drm/sun4i: fix build failure with CONFIG_DRM_SUN8I_MIXER=m
On 11/07/18 15:43, Arnd Bergmann wrote: > Having DRM_SUN4I built-in but DRM_SUN8I_MIXER as a loadable module results in > a link error, as we try to access a symbol from the sun8i_tcon_top.ko module: > > ERROR: "sun8i_tcon_top_of_table" [drivers/gpu/drm/sun4i/sun8i-drm-hdmi.ko] > undefined! > ERROR: "sun8i_tcon_top_of_table" [drivers/gpu/drm/sun4i/sun4i-drm.ko] > undefined! > > This solves the problem by adding a silent symbol for the tcon_top module, > building it as a separate module in exactly the cases that we need it, > but in a way that it is reachable by the other modules. > > Fixes: 57e23de02f48 ("drm/sun4i: DW HDMI: Expand algorithm for possible > crtcs") > Fixes: ef0cf6441fbb ("drm/sun4i: Add support for traversing graph with TCON > TOP") > Signed-off-by: Arnd Bergmann I am seeing the following on today's -next (20180907) as well the last few -next versions for that matter ... ERROR: "sun8i_tcon_top_de_config" [drivers/gpu/drm/sun4i/sun4i-tcon.ko] undefined! ERROR: "sun8i_tcon_top_set_hdmi_src" [drivers/gpu/drm/sun4i/sun4i-tcon.ko] undefined! ERROR: "sun8i_tcon_top_of_table" [drivers/gpu/drm/sun4i/sun4i-tcon.ko] undefined! Seems like this issue has cropped up again as Arnd's fix is present. I am seeing this on ARM64 builds. Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [v3] drm/sun4i: fix build failure with CONFIG_DRM_SUN8I_MIXER=m
On 07/09/18 12:42, Maxime Ripard wrote: > On Fri, Sep 07, 2018 at 01:26:30PM +0200, Arnd Bergmann wrote: >> On Fri, Sep 7, 2018 at 11:41 AM Jon Hunter wrote: >>> >>> >>> On 11/07/18 15:43, Arnd Bergmann wrote: >>>> Having DRM_SUN4I built-in but DRM_SUN8I_MIXER as a loadable module results >>>> in >>>> a link error, as we try to access a symbol from the sun8i_tcon_top.ko >>>> module: >>>> >>>> ERROR: "sun8i_tcon_top_of_table" [drivers/gpu/drm/sun4i/sun8i-drm-hdmi.ko] >>>> undefined! >>>> ERROR: "sun8i_tcon_top_of_table" [drivers/gpu/drm/sun4i/sun4i-drm.ko] >>>> undefined! >>>> >>>> This solves the problem by adding a silent symbol for the tcon_top module, >>>> building it as a separate module in exactly the cases that we need it, >>>> but in a way that it is reachable by the other modules. >>>> >>>> Fixes: 57e23de02f48 ("drm/sun4i: DW HDMI: Expand algorithm for possible >>>> crtcs") >>>> Fixes: ef0cf6441fbb ("drm/sun4i: Add support for traversing graph with >>>> TCON TOP") >>>> Signed-off-by: Arnd Bergmann >>> I am seeing the following on today's -next (20180907) as well the last >>> few -next versions for that matter ... >>> >>> ERROR: "sun8i_tcon_top_de_config" [drivers/gpu/drm/sun4i/sun4i-tcon.ko] >>> undefined! >>> ERROR: "sun8i_tcon_top_set_hdmi_src" [drivers/gpu/drm/sun4i/sun4i-tcon.ko] >>> undefined! >>> ERROR: "sun8i_tcon_top_of_table" [drivers/gpu/drm/sun4i/sun4i-tcon.ko] >>> undefined! >>> >>> Seems like this issue has cropped up again as Arnd's fix is present. I >>> am seeing this on ARM64 builds. >> >> I have not started build testing on linux-next since the merge window, but >> looking at the changes that got queued up, I find commits cf77d79b4e29 >> ("drm/sun4i: tcon: Add another way for matching mixers with tcon") and >> 0305189afb32 ("drm/sun4i: tcon: Add support for R40 TCON") that both >> introduced a reference to the tcon_top file from sun4i_tcon.c. >> >> More IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP) checks in >> the caller like I did in my patch would help, or alternatively one could >> decide to give up and just always include the TCON_TOP. > > Sorry for the breakage. > > Can you test: > > 8< > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index 4834c90b4912..c78cd35a1294 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -974,7 +974,8 @@ static bool sun4i_tcon_connected_to_tcon_top(struct > device_node *node) > > remote = of_graph_get_remote_node(node, 0, -1); > if (remote) { > - ret = !!of_match_node(sun8i_tcon_top_of_table, remote); > + ret = !!(IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP) && > + of_match_node(sun8i_tcon_top_of_table, remote)); > of_node_put(remote); > } > > @@ -1402,13 +1403,20 @@ static int sun8i_r40_tcon_tv_set_mux(struct > sun4i_tcon *tcon, > if (!pdev) > return -EINVAL; > > - if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS) { > + if (IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP) && > + encoder->encoder_type == DRM_MODE_ENCODER_TMDS) { > ret = sun8i_tcon_top_set_hdmi_src(>dev, id); > if (ret) > return ret; > } > > - return sun8i_tcon_top_de_config(>dev, tcon->id, id); > + if (IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP)) { > + ret = sun8i_tcon_top_de_config(>dev, tcon->id, id); > + if (ret) > + return ret; > + } > + > + return 0; > } > > static const struct sun4i_tcon_quirks sun4i_a10_quirks = { > 8< Yes works for me! Thanks, Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1] drm/tegra: dpaux: Fix always-failing probing of the driver
On 24/09/18 12:59, Thierry Reding wrote: > On Fri, Sep 21, 2018 at 02:42:41PM +0300, Dmitry Osipenko wrote: >> Some of definitions in the code changed the meaning, unfortunately one >> place missed the change. >> >> Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support") >> Cc: # v4.8+ >> Signed-off-by: Dmitry Osipenko >> --- >> >> I don't have HW to test DPAUX driver, apparently it has been broken for >> 2+ years now. There is also a known issue on with the DPAUX driver that >> prevents it from probing, that was discussed on the #tegra IRC. Thierry, >> please take a closer look at this driver and test it thoroughly, it has >> some obvious problems. >> >> drivers/gpu/drm/tegra/dpaux.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > It's odd that you claim that the driver is always failing probe and at > the same time you say that you don't have hardware to test the driver. > =) > > I know for a fact that this driver does not usually fail because it is > required on all recent chips (Tegra210 and later) to drive HDMI, which > we support on all boards, so it is indeed thoroughly tested. > >> >> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c >> index d84e81ff36ad..ba5681fab73b 100644 >> --- a/drivers/gpu/drm/tegra/dpaux.c >> +++ b/drivers/gpu/drm/tegra/dpaux.c >> @@ -521,7 +521,7 @@ static int tegra_dpaux_probe(struct platform_device >> *pdev) >> * is no possibility to perform the I2C mode configuration in the >> * HDMI path. >> */ >> -err = tegra_dpaux_pad_config(dpaux, DPAUX_HYBRID_PADCTL_MODE_I2C); >> +err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_I2C); >> if (err < 0) >> return err; >> > > If you look at the definitions of both DPAUX_HYBRID_PADCTL_MODE_I2C and > DPAUX_PADCTL_FUNC_I2C, you'll see that both are actually the same, which > is a good explanation for why the driver performs flawlessly. > > That said, your change is obviously correct. I've applied it, but since > it doesn't actually fix anything, and doesn't change anything from a > binary point of view, I've removed the Fixes: and Cc: stable tags. Did you change the subject for the patch because as you mentioned it does not seem related to the change? Otherwise for the fix you can have my ... Acked-by: Jon Hunter The dpaux driver has been working fine for me on Tegra210 Smaug when using the pins for I2C and I have not seen any probing problems. Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/tegra: vic: Implement explicit reset support
On 23/11/2018 12:06, Thierry Reding wrote: > From: Thierry Reding > > Tegra supports generic PM domains on 64-bit ARM, and if that is enabled, > the power domain code will make sure that resets are asserted and > deasserted at appropriate points in time. > > If generic PM domains are not implemented, such as on 32-bit Tegra, the > resets need to be asserted and deasserted explicitly by the driver. > > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/tegra/vic.c | 35 ++- > 1 file changed, 34 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c > index 9fa77405db01..23f530db45ad 100644 > --- a/drivers/gpu/drm/tegra/vic.c > +++ b/drivers/gpu/drm/tegra/vic.c > @@ -38,6 +38,7 @@ struct vic { > struct iommu_domain *domain; > struct device *dev; > struct clk *clk; > + struct reset_control *rst; > > /* Platform configuration */ > const struct vic_config *config; > @@ -56,13 +57,37 @@ static void vic_writel(struct vic *vic, u32 value, > unsigned int offset) > static int vic_runtime_resume(struct device *dev) > { > struct vic *vic = dev_get_drvdata(dev); > + int err; > + > + err = clk_prepare_enable(vic->clk); > + if (err < 0) > + return err; > + > + usleep_range(2000, 4000); The Tegra genpd code has a usleep_range(10, 20), is that not sufficient here? If it is, it would be good to be consistent. > + > + err = reset_control_deassert(vic->rst); > + if (err < 0) > + goto disable; > + > + usleep_range(2000, 4000); > + > + return 0; > > - return clk_prepare_enable(vic->clk); > +disable: > + clk_disable_unprepare(vic->clk); > + return err; > } > > static int vic_runtime_suspend(struct device *dev) > { > struct vic *vic = dev_get_drvdata(dev); > + int err; > + > + err = reset_control_assert(vic->rst); > + if (err < 0) > + return err; > + > + usleep_range(2000, 4000); > > clk_disable_unprepare(vic->clk); > > @@ -324,6 +349,14 @@ static int vic_probe(struct platform_device *pdev) > return PTR_ERR(vic->clk); > } > > + if (!dev->pm_domain) { > + vic->rst = devm_reset_control_get(dev, "vic"); > + if (IS_ERR(vic->rst)) { > + dev_err(>dev, "failed to get reset\n"); > + return PTR_ERR(vic->rst); > + } > + } > + > vic->falcon.dev = dev; > vic->falcon.regs = vic->regs; > vic->falcon.ops = _falcon_ops; > Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/tegra: vic: Implement explicit reset support
On 29/11/2018 14:51, Thierry Reding wrote: > On Thu, Nov 29, 2018 at 01:40:32PM +0000, Jon Hunter wrote: >> >> On 23/11/2018 12:06, Thierry Reding wrote: >>> From: Thierry Reding >>> >>> Tegra supports generic PM domains on 64-bit ARM, and if that is enabled, >>> the power domain code will make sure that resets are asserted and >>> deasserted at appropriate points in time. >>> >>> If generic PM domains are not implemented, such as on 32-bit Tegra, the >>> resets need to be asserted and deasserted explicitly by the driver. >>> >>> Signed-off-by: Thierry Reding >>> --- >>> drivers/gpu/drm/tegra/vic.c | 35 ++- >>> 1 file changed, 34 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c >>> index 9fa77405db01..23f530db45ad 100644 >>> --- a/drivers/gpu/drm/tegra/vic.c >>> +++ b/drivers/gpu/drm/tegra/vic.c >>> @@ -38,6 +38,7 @@ struct vic { >>> struct iommu_domain *domain; >>> struct device *dev; >>> struct clk *clk; >>> + struct reset_control *rst; >>> >>> /* Platform configuration */ >>> const struct vic_config *config; >>> @@ -56,13 +57,37 @@ static void vic_writel(struct vic *vic, u32 value, >>> unsigned int offset) >>> static int vic_runtime_resume(struct device *dev) >>> { >>> struct vic *vic = dev_get_drvdata(dev); >>> + int err; >>> + >>> + err = clk_prepare_enable(vic->clk); >>> + if (err < 0) >>> + return err; >>> + >>> + usleep_range(2000, 4000); >> >> The Tegra genpd code has a usleep_range(10, 20), is that not sufficient >> here? If it is, it would be good to be consistent. > > Yeah, I think that's enough. The Tegra DRM driver uses these ranges in > many places, so that's where I copied them from. None of these are part > of a hot path or anything, so whether this sleeps for 10 ns or 4 ms is > not going to matter much. > > With that changed, can I consider this R-b you? Yes please add my r-b. Thanks Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "dma-contiguous: do not allocate a single page from CMA area"
On 26/02/2019 20:23, Nicolin Chen wrote: > This reverts commit d222e42e88168fd67e6d131984b86477af1fc256. > > The original change breaks omap dss: > omapdss_dispc 58001000.dispc: > dispc_errata_i734_wa_init: dma_alloc_writecombine failed > > Let's revert it first and then find a safer solution instead. > > Reported-by: Tony Lindgren > Signed-off-by: Nicolin Chen > --- > Tony, > > Would you please test and verify? Thanks! This also fixes various memory allocation failures we have seen on 32-bit Tegra as well. Tested-by: Jon Hunter Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm connectors, tegra, and the web they weave (was Re: [PATCH 58/59] drm/todo: Add new debugfs todo)
On 18/06/2019 16:19, Greg Kroah-Hartman wrote: > On Fri, Jun 14, 2019 at 10:36:14PM +0200, Daniel Vetter wrote: >> Greg is busy already, but maybe he won't do everything ... >> >> Cc: Greg Kroah-Hartman >> Signed-off-by: Daniel Vetter >> --- >> Documentation/gpu/todo.rst | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst >> index 9717540ee28f..026e55c517e1 100644 >> --- a/Documentation/gpu/todo.rst >> +++ b/Documentation/gpu/todo.rst >> @@ -375,6 +375,9 @@ There's a bunch of issues with it: >>this (together with the drm_minor->drm_device move) would allow us to >> remove >>debugfs_init. >> >> +- Drop the return code and error checking from all debugfs functions. Greg >> KH is >> + working on this already. > > > Part of this work was to try to delete drm_debugfs_remove_files(). > > There are only 4 files that currently still call this function: > drivers/gpu/drm/tegra/dc.c > drivers/gpu/drm/tegra/dsi.c > drivers/gpu/drm/tegra/hdmi.c > drivers/gpu/drm/tegra/sor.c > > For dc.c, the driver wants to add debugfs files to the struct drm_crtc > debugfs directory. Which is fine, but it has to do some special memory > allocation to get the debugfs callback to point not to the struct > drm_minor pointer, but rather the drm_crtc structure. > > So, to remove this call, I need to remove this special memory allocation > and to do that, I need to somehow be able to cast from drm_minor back to > the drm_crtc structure being used in this driver. And I can't figure > how they are related at all. > > Any pointers here (pun intended) would be appreciated. > > For the other 3 files, the situation is much the same, but I need to get > from a 'struct drm_minor' pointer to a 'struct drm_connector' pointer. > > I could just "open code" a bunch of calls to debugfs_create_file() for > these drivers, which would solve this issue, but in a more "non-drm" > way. Is it worth to just do that instead of overthinking the whole > thing and trying to squish it into the drm "model" of drm debugfs calls? > > Either way, who can test these changes? I can't even build the tegra > driver without digging up an arm64 cross-compiler, and can't test it as > I have no hardware at all. We can definitely compile and boot test these no problem. In fact anything that lands in -next we will boot test. However, I can do some quick sanity if you have something to test. Thierry may have more specific Tegra DRM tests. Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 05/12] drm/modes: Rewrite the command line parser
On 09/07/2019 13:52, Dmitry Osipenko wrote: > 09.07.2019 15:45, Maxime Ripard пишет: >> Hi, >> >> On Fri, Jul 05, 2019 at 07:54:47PM +0300, Dmitry Osipenko wrote: >>> 17.06.2019 17:51, Maxime Ripard пишет: From: Maxime Ripard Rewrite the command line parser in order to get away from the state machine parsing the video mode lines. Hopefully, this will allow to extend it more easily to support named modes and / or properties set directly on the command line. Reviewed-by: Noralf Trønnes Signed-off-by: Maxime Ripard --- drivers/gpu/drm/drm_modes.c | 325 +++-- 1 file changed, 210 insertions(+), 115 deletions(-) >>> >>> I have a Tegra device that uses a stock android bootloader which >>> passes "video=tegrafb" in the kernels cmdline. That wasn't a problem >>> before this patch, but now Tegra DRM driver fails to probe because >>> the mode is 0x0:0 and hence framebuffer allocation fails. Is it a >>> legit regression that should be fixed in upstream? >> >> Thierry indeed reported that issue, but the discussion pretty much >> stalled since then. Yes Thierry is currently out and hence has not responded. I had been planning to look at this this week and responded. > Sorry, this doesn't answer my question. Where it was reported? Same thread [0]. Dmitry, are you able to respond to Maxime's response to Thierry? Cheers Jon [0] https://patchwork.kernel.org/patch/10999393/ -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 05/12] drm/modes: Rewrite the command line parser
On 09/07/2019 14:26, Jon Hunter wrote: > > On 09/07/2019 13:52, Dmitry Osipenko wrote: >> 09.07.2019 15:45, Maxime Ripard пишет: >>> Hi, >>> >>> On Fri, Jul 05, 2019 at 07:54:47PM +0300, Dmitry Osipenko wrote: >>>> 17.06.2019 17:51, Maxime Ripard пишет: >>>>> From: Maxime Ripard >>>>> >>>>> Rewrite the command line parser in order to get away from the state >>>>> machine >>>>> parsing the video mode lines. >>>>> >>>>> Hopefully, this will allow to extend it more easily to support named modes >>>>> and / or properties set directly on the command line. >>>>> >>>>> Reviewed-by: Noralf Trønnes >>>>> Signed-off-by: Maxime Ripard >>>>> --- >>>>> drivers/gpu/drm/drm_modes.c | 325 +++-- >>>>> 1 file changed, 210 insertions(+), 115 deletions(-) >>>> >>>> I have a Tegra device that uses a stock android bootloader which >>>> passes "video=tegrafb" in the kernels cmdline. That wasn't a problem >>>> before this patch, but now Tegra DRM driver fails to probe because >>>> the mode is 0x0:0 and hence framebuffer allocation fails. Is it a >>>> legit regression that should be fixed in upstream? >>> >>> Thierry indeed reported that issue, but the discussion pretty much >>> stalled since then. > > Yes Thierry is currently out and hence has not responded. I had been > planning to look at this this week and responded. > >> Sorry, this doesn't answer my question. Where it was reported? > > Same thread [0]. Correction, it was on patch 6/12 and not this one. Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/tegra: sor: Enable HDA interrupts at plug-in
On 24/07/2019 10:27, Dmitry Osipenko wrote: > 23.07.2019 15:40, Viswanath L пишет: >> HDMI plugout calls runtime suspend, which clears interrupt registers >> and causes audio functionality to break on subsequent plug-in; setting >> interrupt registers in sor_audio_prepare() solves the issue. >> >> Signed-off-by: Viswanath L > > Yours signed-off-by always should be the last line of the commit's > message because the text below it belongs to a person who applies this > patch, Thierry in this case. This is not a big deal at all and Thierry > could make a fixup while applying the patch if will deem that as necessary. > > Secondly, there is no need to add "sta...@vger.kernel.org" to the > email's recipients because the patch will flow into stable kernel > versions from the mainline once it will get applied. That happens based > on the stable tag presence, hence it's enough to add the 'Cc' tag to the > commit's message in order to get patch backported. I believe 'git send-email' automatically does this. Jon -- nvpublic
Re: [PATCH v1] drm/tegra: Fix gpiod_get_from_of_node() regression
On 05/07/2019 16:11, Dmitry Osipenko wrote: > That function now returns ERR_PTR instead of NULL if "hpd-gpio" is not > present in device-tree. The offending patch missed to adapt the Tegra's > DRM driver for the API change. > > Fixes: 025bf37725f1 ("gpio: Fix return value mismatch of function > gpiod_get_from_of_node()") > Signed-off-by: Dmitry Osipenko > --- > drivers/gpu/drm/tegra/output.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c > index 274cb955e2e1..471d33809cd4 100644 > --- a/drivers/gpu/drm/tegra/output.c > +++ b/drivers/gpu/drm/tegra/output.c > @@ -126,8 +126,12 @@ int tegra_output_probe(struct tegra_output *output) > "nvidia,hpd-gpio", 0, > GPIOD_IN, > "HDMI hotplug detect"); > - if (IS_ERR(output->hpd_gpio)) > - return PTR_ERR(output->hpd_gpio); > + if (IS_ERR(output->hpd_gpio)) { > + if (PTR_ERR(output->hpd_gpio) == -ENOENT) > + output->hpd_gpio = NULL; > + else > + return PTR_ERR(output->hpd_gpio); > + } > > if (output->hpd_gpio) { > err = gpiod_to_irq(output->hpd_gpio); > Acked-by: Jon Hunter Cheers Jon -- nvpublic
[PATCH] backlight: lp855x: Ensure regulators are disabled on probe failure
If probing the LP885x backlight fails after the regulators have been enabled, then the following warning is seen when releasing the regulators ... WARNING: CPU: 1 PID: 289 at drivers/regulator/core.c:2051 _regulator_put.part.28+0x158/0x160 Modules linked in: tegra_xudc lp855x_bl(+) host1x pwm_tegra ip_tables x_tables ipv6 nf_defrag_ipv6 CPU: 1 PID: 289 Comm: systemd-udevd Not tainted 5.6.0-rc2-next-20200224 #1 Hardware name: NVIDIA Jetson TX1 Developer Kit (DT) ... Call trace: _regulator_put.part.28+0x158/0x160 regulator_put+0x34/0x50 devm_regulator_release+0x10/0x18 release_nodes+0x12c/0x230 devres_release_all+0x34/0x50 really_probe+0x1c0/0x370 driver_probe_device+0x58/0x100 device_driver_attach+0x6c/0x78 __driver_attach+0xb0/0xf0 bus_for_each_dev+0x68/0xc8 driver_attach+0x20/0x28 bus_add_driver+0x160/0x1f0 driver_register+0x60/0x110 i2c_register_driver+0x40/0x80 lp855x_driver_init+0x20/0x1000 [lp855x_bl] do_one_initcall+0x58/0x1a0 do_init_module+0x54/0x1d0 load_module+0x1d80/0x21c8 __do_sys_finit_module+0xe8/0x100 __arm64_sys_finit_module+0x18/0x20 el0_svc_common.constprop.3+0xb0/0x168 do_el0_svc+0x20/0x98 el0_sync_handler+0xf4/0x1b0 el0_sync+0x140/0x180 Fix this by ensuring that the regulators are disabled, if enabled, on probe failure. Finally, ensure that the vddio regulator is disabled in the driver remove handler. Signed-off-by: Jon Hunter --- drivers/video/backlight/lp855x_bl.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c index f68920131a4a..e94932c69f54 100644 --- a/drivers/video/backlight/lp855x_bl.c +++ b/drivers/video/backlight/lp855x_bl.c @@ -456,7 +456,7 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) ret = regulator_enable(lp->enable); if (ret < 0) { dev_err(lp->dev, "failed to enable vddio: %d\n", ret); - return ret; + goto disable_supply; } /* @@ -471,24 +471,34 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id) ret = lp855x_configure(lp); if (ret) { dev_err(lp->dev, "device config err: %d", ret); - return ret; + goto disable_vddio; } ret = lp855x_backlight_register(lp); if (ret) { dev_err(lp->dev, "failed to register backlight. err: %d\n", ret); - return ret; + goto disable_vddio; } ret = sysfs_create_group(>dev->kobj, _attr_group); if (ret) { dev_err(lp->dev, "failed to register sysfs. err: %d\n", ret); - return ret; + goto disable_vddio; } backlight_update_status(lp->bl); + return 0; + +disable_vddio: + if (lp->enable) + regulator_disable(lp->enable); +disable_supply: + if (lp->supply) + regulator_disable(lp->supply); + + return ret; } static int lp855x_remove(struct i2c_client *cl) @@ -497,6 +507,8 @@ static int lp855x_remove(struct i2c_client *cl) lp->bl->props.brightness = 0; backlight_update_status(lp->bl); + if (lp->enable) + regulator_disable(lp->enable); if (lp->supply) regulator_disable(lp->supply); sysfs_remove_group(>dev->kobj, _attr_group); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: lp855x: Ensure regulators are disabled on probe failure
Hi Lee, Daniel, On 24/02/2020 14:37, Daniel Thompson wrote: > On Mon, Feb 24, 2020 at 02:07:48PM +0000, Jon Hunter wrote: >> If probing the LP885x backlight fails after the regulators have been >> enabled, then the following warning is seen when releasing the >> regulators ... >> >> WARNING: CPU: 1 PID: 289 at drivers/regulator/core.c:2051 >> _regulator_put.part.28+0x158/0x160 >> Modules linked in: tegra_xudc lp855x_bl(+) host1x pwm_tegra ip_tables >> x_tables ipv6 nf_defrag_ipv6 >> CPU: 1 PID: 289 Comm: systemd-udevd Not tainted 5.6.0-rc2-next-20200224 #1 >> Hardware name: NVIDIA Jetson TX1 Developer Kit (DT) >> >> ... >> >> Call trace: >> _regulator_put.part.28+0x158/0x160 >> regulator_put+0x34/0x50 >> devm_regulator_release+0x10/0x18 >> release_nodes+0x12c/0x230 >> devres_release_all+0x34/0x50 >> really_probe+0x1c0/0x370 >> driver_probe_device+0x58/0x100 >> device_driver_attach+0x6c/0x78 >> __driver_attach+0xb0/0xf0 >> bus_for_each_dev+0x68/0xc8 >> driver_attach+0x20/0x28 >> bus_add_driver+0x160/0x1f0 >> driver_register+0x60/0x110 >> i2c_register_driver+0x40/0x80 >> lp855x_driver_init+0x20/0x1000 [lp855x_bl] >> do_one_initcall+0x58/0x1a0 >> do_init_module+0x54/0x1d0 >> load_module+0x1d80/0x21c8 >> __do_sys_finit_module+0xe8/0x100 >> __arm64_sys_finit_module+0x18/0x20 >> el0_svc_common.constprop.3+0xb0/0x168 >> do_el0_svc+0x20/0x98 >> el0_sync_handler+0xf4/0x1b0 >> el0_sync+0x140/0x180 >> >> Fix this by ensuring that the regulators are disabled, if enabled, on >> probe failure. >> >> Finally, ensure that the vddio regulator is disabled in the driver >> remove handler. >> >> Signed-off-by: Jon Hunter > > Reviewed-by: Daniel Thompson I received a bounce from Milo's email and so I am not sure that his email address is still valid. Can either of you pick this up? Not sure if we should update the MAINTAINERS as well? Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] gpu: host1x: Use SMMU on Tegra124 and Tegra210
On 25/03/2020 20:16, Thierry Reding wrote: > From: Thierry Reding > > Tegra124 and Tegra210 support addressing more than 32 bits of physical > memory. However, since their host1x does not support the wide GATHER > opcode, they should use the SMMU if at all possible to ensure that all > the system memory can be used for command buffers, irrespective of > whether or not the host1x firewall is enabled. > > Signed-off-by: Thierry Reding > --- > drivers/gpu/host1x/dev.c | 46 > 1 file changed, 42 insertions(+), 4 deletions(-) Tested-by: Jon Hunter Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/tegra: Fix SMMU support on Tegra124 and Tegra210
On 25/03/2020 20:16, Thierry Reding wrote: > From: Thierry Reding > > When testing whether or not to enable the use of the SMMU, consult the > supported DMA mask rather than the actually configured DMA mask, since > the latter might already have been restricted. > > Fixes: 2d9384ff9177 ("drm/tegra: Relax IOMMU usage criteria on old Tegra") > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/tegra/drm.c | 3 ++- > drivers/gpu/host1x/dev.c| 13 + > include/linux/host1x.h | 3 +++ > 3 files changed, 18 insertions(+), 1 deletion(-) Tested-by: Jon Hunter Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: lp855x: Ensure regulators are disabled on probe failure
Hi Lee, On 16/03/2020 09:05, Daniel Thompson wrote: > On Fri, Mar 13, 2020 at 02:16:16PM +0000, Jon Hunter wrote: >> Hi Lee, Daniel, >> >> On 24/02/2020 14:37, Daniel Thompson wrote: >>> On Mon, Feb 24, 2020 at 02:07:48PM +, Jon Hunter wrote: >>>> If probing the LP885x backlight fails after the regulators have been >>>> enabled, then the following warning is seen when releasing the >>>> regulators ... >>>> >>>> WARNING: CPU: 1 PID: 289 at drivers/regulator/core.c:2051 >>>> _regulator_put.part.28+0x158/0x160 >>>> Modules linked in: tegra_xudc lp855x_bl(+) host1x pwm_tegra ip_tables >>>> x_tables ipv6 nf_defrag_ipv6 >>>> CPU: 1 PID: 289 Comm: systemd-udevd Not tainted 5.6.0-rc2-next-20200224 #1 >>>> Hardware name: NVIDIA Jetson TX1 Developer Kit (DT) >>>> >>>> ... >>>> >>>> Fix this by ensuring that the regulators are disabled, if enabled, on >>>> probe failure. >>>> >>>> Finally, ensure that the vddio regulator is disabled in the driver >>>> remove handler. >>>> >>>> Signed-off-by: Jon Hunter >>> >>> Reviewed-by: Daniel Thompson >> I received a bounce from Milo's email and so I am not sure that his >> email address is still valid. >> >> Can either of you pick this up? > > Lee generally starts to hoover up patches about this stage in the dev > cycle so I'd expect this to move fairly soon. Does not look like this ever got picked up. Please let me know if you can queue this one. Note it still applies cleanly to -next. Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 17/19] ARM: tegra: Add EMC OPP properties to Tegra20 device-trees
On 30/11/2020 22:57, Dmitry Osipenko wrote: > 01.12.2020 00:17, Jon Hunter пишет: >> Hi Dmitry, >> >> On 23/11/2020 00:27, Dmitry Osipenko wrote: >>> Add EMC OPP DVFS tables and update board device-trees by removing >>> unsupported OPPs. >>> >>> Signed-off-by: Dmitry Osipenko >> This change is generating the following warning on Tegra20 Ventana >> and prevents the EMC from probing ... >> >> [2.485711] tegra20-emc 7000f400.memory-controller: device-tree doesn't >> have memory timings >> [2.499386] tegra20-emc 7000f400.memory-controller: 32bit DRAM bus >> [2.505810] [ cut here ] >> [2.510511] WARNING: CPU: 0 PID: 1 at >> /local/workdir/tegra/mlt-linux_next/kernel/drivers/opp/of.c:875 >> _of_add_opp_table_v2+0x598/0x61c >> [2.529746] Modules linked in: >> [2.540140] CPU: 0 PID: 1 Comm: swapper/0 Not tainted >> 5.10.0-rc5-next-20201130 #1 >> [2.554606] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) >> [2.560892] [] (unwind_backtrace) from [] >> (show_stack+0x10/0x14) >> [2.568640] [] (show_stack) from [] >> (dump_stack+0xc8/0xdc) >> [2.575866] [] (dump_stack) from [] >> (__warn+0x104/0x108) >> [2.582912] [] (__warn) from [] >> (warn_slowpath_fmt+0xb0/0xb8) >> [2.590397] [] (warn_slowpath_fmt) from [] >> (_of_add_opp_table_v2+0x598/0x61c) >> [2.599269] [] (_of_add_opp_table_v2) from [] >> (dev_pm_opp_of_add_table+0x3c/0x1a0) >> [2.608582] [] (dev_pm_opp_of_add_table) from [] >> (tegra_emc_probe+0x478/0x940) >> [2.617548] [] (tegra_emc_probe) from [] >> (platform_drv_probe+0x48/0x98) >> [2.625899] [] (platform_drv_probe) from [] >> (really_probe+0x218/0x3b8) >> [2.634162] [] (really_probe) from [] >> (driver_probe_device+0x5c/0xb4) >> [2.642338] [] (driver_probe_device) from [] >> (device_driver_attach+0x58/0x60) >> [2.651208] [] (device_driver_attach) from [] >> (__driver_attach+0x80/0xbc) >> [2.659730] [] (__driver_attach) from [] >> (bus_for_each_dev+0x74/0xb4) >> [2.667905] [] (bus_for_each_dev) from [] >> (bus_add_driver+0x164/0x1e8) >> [2.676168] [] (bus_add_driver) from [] >> (driver_register+0x7c/0x114) >> [2.684259] [] (driver_register) from [] >> (do_one_initcall+0x54/0x2b0) >> [2.692441] [] (do_one_initcall) from [] >> (kernel_init_freeable+0x1a4/0x1f4) >> [2.701145] [] (kernel_init_freeable) from [] >> (kernel_init+0x8/0x118) >> [2.709321] [] (kernel_init) from [] >> (ret_from_fork+0x14/0x24) >> [2.716885] Exception stack(0xc1501fb0 to 0xc1501ff8) >> [2.721933] 1fa0: >> >> [2.730106] 1fc0: >> >> [2.738278] 1fe0: 0013 >> [2.751940] ---[ end trace 61e3b76deca27ef3 ]--- >> >> >> Cheers >> Jon >> > > Hello Jon, > > That is harmless and expected to happen because the patch "memory: > tegra20: Support hardware versioning and clean up OPP table > initialization" isn't applied yet, while Thierry already applied the DT > patches from this v10. Thanks, pulling in that patch did fix it. Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 17/19] ARM: tegra: Add EMC OPP properties to Tegra20 device-trees
Hi Dmitry, On 23/11/2020 00:27, Dmitry Osipenko wrote: > Add EMC OPP DVFS tables and update board device-trees by removing > unsupported OPPs. > > Signed-off-by: Dmitry Osipenko This change is generating the following warning on Tegra20 Ventana and prevents the EMC from probing ... [2.485711] tegra20-emc 7000f400.memory-controller: device-tree doesn't have memory timings [2.499386] tegra20-emc 7000f400.memory-controller: 32bit DRAM bus [2.505810] [ cut here ] [2.510511] WARNING: CPU: 0 PID: 1 at /local/workdir/tegra/mlt-linux_next/kernel/drivers/opp/of.c:875 _of_add_opp_table_v2+0x598/0x61c [2.529746] Modules linked in: [2.540140] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.10.0-rc5-next-20201130 #1 [2.554606] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [2.560892] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [2.568640] [] (show_stack) from [] (dump_stack+0xc8/0xdc) [2.575866] [] (dump_stack) from [] (__warn+0x104/0x108) [2.582912] [] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8) [2.590397] [] (warn_slowpath_fmt) from [] (_of_add_opp_table_v2+0x598/0x61c) [2.599269] [] (_of_add_opp_table_v2) from [] (dev_pm_opp_of_add_table+0x3c/0x1a0) [2.608582] [] (dev_pm_opp_of_add_table) from [] (tegra_emc_probe+0x478/0x940) [2.617548] [] (tegra_emc_probe) from [] (platform_drv_probe+0x48/0x98) [2.625899] [] (platform_drv_probe) from [] (really_probe+0x218/0x3b8) [2.634162] [] (really_probe) from [] (driver_probe_device+0x5c/0xb4) [2.642338] [] (driver_probe_device) from [] (device_driver_attach+0x58/0x60) [2.651208] [] (device_driver_attach) from [] (__driver_attach+0x80/0xbc) [2.659730] [] (__driver_attach) from [] (bus_for_each_dev+0x74/0xb4) [2.667905] [] (bus_for_each_dev) from [] (bus_add_driver+0x164/0x1e8) [2.676168] [] (bus_add_driver) from [] (driver_register+0x7c/0x114) [2.684259] [] (driver_register) from [] (do_one_initcall+0x54/0x2b0) [2.692441] [] (do_one_initcall) from [] (kernel_init_freeable+0x1a4/0x1f4) [2.701145] [] (kernel_init_freeable) from [] (kernel_init+0x8/0x118) [2.709321] [] (kernel_init) from [] (ret_from_fork+0x14/0x24) [2.716885] Exception stack(0xc1501fb0 to 0xc1501ff8) [2.721933] 1fa0: [2.730106] 1fc0: [2.738278] 1fe0: 0013 [2.751940] ---[ end trace 61e3b76deca27ef3 ]--- Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/tegra: sor: Don't warn on probe deferral
Deferred probe is an expected return value for tegra_output_probe(). Given that the driver deals with it properly, there's no need to output a warning that may potentially confuse users. Signed-off-by: Jon Hunter --- drivers/gpu/drm/tegra/sor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index e88a17c2937f..5a232055b8cc 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -3765,7 +3765,7 @@ static int tegra_sor_probe(struct platform_device *pdev) err = tegra_output_probe(>output); if (err < 0) { - dev_err(>dev, "failed to probe output: %d\n", err); + dev_err_probe(>dev, "failed to probe output: %d\n", err); return err; } -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/tegra: sor: Don't warn on probe deferral
On 03/11/2020 11:44, Jon Hunter wrote: > Deferred probe is an expected return value for tegra_output_probe(). > Given that the driver deals with it properly, there's no need to output > a warning that may potentially confuse users. > > Signed-off-by: Jon Hunter > --- > drivers/gpu/drm/tegra/sor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c > index e88a17c2937f..5a232055b8cc 100644 > --- a/drivers/gpu/drm/tegra/sor.c > +++ b/drivers/gpu/drm/tegra/sor.c > @@ -3765,7 +3765,7 @@ static int tegra_sor_probe(struct platform_device *pdev) > > err = tegra_output_probe(>output); > if (err < 0) { > - dev_err(>dev, "failed to probe output: %d\n", err); > + dev_err_probe(>dev, "failed to probe output: %d\n", err); > return err; > } Sorry this is not right. I will fix this in V2. Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V2] drm/tegra: sor: Don't warn on probe deferral
On 04/11/2020 10:49, Dmitry Osipenko wrote: > 04.11.2020 12:23, Jon Hunter пишет: >> Deferred probe is an expected return value for tegra_output_probe(). >> Given that the driver deals with it properly, there's no need to output >> a warning that may potentially confuse users. >> >> Signed-off-by: Jon Hunter >> --- >> >> Changes since V1: >> - This time, I actually validated it! >> >> drivers/gpu/drm/tegra/sor.c | 7 +++ >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c >> index e88a17c2937f..898a80ca37fa 100644 >> --- a/drivers/gpu/drm/tegra/sor.c >> +++ b/drivers/gpu/drm/tegra/sor.c >> @@ -3764,10 +3764,9 @@ static int tegra_sor_probe(struct platform_device >> *pdev) >> return err; >> >> err = tegra_output_probe(>output); >> -if (err < 0) { >> -dev_err(>dev, "failed to probe output: %d\n", err); >> -return err; >> -} >> +if (err < 0) >> +return dev_err_probe(>dev, err, >> + "failed to probe output: %d\n", err); > > Hello Jon, > > There is no need to duplicate the error code in the message [1]. Perhaps > worth making a v3? :) Indeed! Thanks for catching. Trying to do to many things at the same time. I should have learned by now! Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V3] drm/tegra: sor: Don't warn on probe deferral
Deferred probe is an expected return value for tegra_output_probe(). Given that the driver deals with it properly, there's no need to output a warning that may potentially confuse users. Signed-off-by: Jon Hunter --- Changes since V2: - Removed duplicate errno print Changes since V1: - This time, I actually validated it! drivers/gpu/drm/tegra/sor.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index e88a17c2937f..a5b944dacb35 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -3764,10 +3764,9 @@ static int tegra_sor_probe(struct platform_device *pdev) return err; err = tegra_output_probe(>output); - if (err < 0) { - dev_err(>dev, "failed to probe output: %d\n", err); - return err; - } + if (err < 0) + return dev_err_probe(>dev, err, +"failed to probe output\n"); if (sor->ops && sor->ops->probe) { err = sor->ops->probe(sor); -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V2] drm/tegra: sor: Don't warn on probe deferral
Deferred probe is an expected return value for tegra_output_probe(). Given that the driver deals with it properly, there's no need to output a warning that may potentially confuse users. Signed-off-by: Jon Hunter --- Changes since V1: - This time, I actually validated it! drivers/gpu/drm/tegra/sor.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index e88a17c2937f..898a80ca37fa 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -3764,10 +3764,9 @@ static int tegra_sor_probe(struct platform_device *pdev) return err; err = tegra_output_probe(>output); - if (err < 0) { - dev_err(>dev, "failed to probe output: %d\n", err); - return err; - } + if (err < 0) + return dev_err_probe(>dev, err, +"failed to probe output: %d\n", err); if (sor->ops && sor->ops->probe) { err = sor->ops->probe(sor); -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 13/15] drm/tegra: Implement job submission part of new UAPI
Hi Mikko, On 10/06/2021 12:04, Mikko Perttunen wrote: > Implement the job submission IOCTL with a minimum feature set. > > Signed-off-by: Mikko Perttunen > --- > v7: > * Allocate gather BO with DMA API to get page-aligned > memory > * Add error prints to a few places where they were missing > v6: > * Remove sgt bypass path in gather_bo - this would cause > cache maintenance to be skipped and is unnecessary in > general. > * Changes related to moving to using syncpoint IDs > * Add syncobj related code > * Print warning on submit failure describing the issue > * Use ARCH_DMA_ADDR_T_64BIT to check if that is indeed > the case > * Add support for relative syncpoint wait > * Use pm_runtime_resume_and_get > * Only try to resume engines that support runtime PM > * Removed uapi subdirectory > * Don't use "copy_err" variables for copy_from_user > return value > * Fix setting of blocklinear flag > v5: > * Add 16K size limit to copies from userspace. > * Guard RELOC_BLOCKLINEAR flag handling to only exist in ARM64 > to prevent oversized shift on 32-bit platforms. > v4: > * Remove all features that are not strictly necessary. > * Split into two patches. > v3: > * Remove WRITE_RELOC. Relocations are now patched implicitly > when patching is needed. > * Directly call PM runtime APIs on devices instead of using > power_on/power_off callbacks. > * Remove incorrect mutex unlock in tegra_drm_ioctl_channel_open > * Use XA_FLAGS_ALLOC1 instead of XA_FLAGS_ALLOC > * Accommodate for removal of timeout field and inlining of > syncpt_incrs array. > * Copy entire user arrays at a time instead of going through > elements one-by-one. > * Implement waiting of DMA reservations. > * Split out gather_bo implementation into a separate file. > * Fix length parameter passed to sg_init_one in gather_bo > * Cosmetic cleanup. > --- > drivers/gpu/drm/tegra/Makefile| 2 + > drivers/gpu/drm/tegra/drm.c | 4 +- > drivers/gpu/drm/tegra/gather_bo.c | 82 + > drivers/gpu/drm/tegra/gather_bo.h | 24 ++ > drivers/gpu/drm/tegra/submit.c| 549 ++ > drivers/gpu/drm/tegra/submit.h| 17 + > 6 files changed, 677 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/tegra/gather_bo.c > create mode 100644 drivers/gpu/drm/tegra/gather_bo.h > create mode 100644 drivers/gpu/drm/tegra/submit.c > create mode 100644 drivers/gpu/drm/tegra/submit.h ... > diff --git a/drivers/gpu/drm/tegra/submit.c b/drivers/gpu/drm/tegra/submit.c > new file mode 100644 > index ..e3200c10ca9e > --- /dev/null > +++ b/drivers/gpu/drm/tegra/submit.c > @@ -0,0 +1,549 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright (c) 2020 NVIDIA Corporation */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +#include "drm.h" > +#include "gather_bo.h" > +#include "gem.h" > +#include "submit.h" > +#include "uapi.h" > + > +#define SUBMIT_ERR(ctx, fmt, ...) \ > + dev_err_ratelimited(ctx->client->base.dev, \ > + "%s: job submission failed: " fmt "\n", \ > + current->comm __VA_OPT__(,) __VA_ARGS__) For older compilers that don't support __VA_OPT__ this generates a compilation error ... drivers/gpu/drm/tegra/submit.c: In function ‘submit_copy_gather_data’: drivers/gpu/drm/tegra/submit.c:27:17: error: expected ‘)’ before ‘__VA_OPT__’ current->comm __VA_OPT__(,) __VA_ARGS__) ^ I think that we may just have to use ##__VA_ARGS__ here. Cheers Jon -- nvpublic
Re: [PATCH v7 13/15] drm/tegra: Implement job submission part of new UAPI
On 10/06/2021 12:04, Mikko Perttunen wrote: > Implement the job submission IOCTL with a minimum feature set. > > Signed-off-by: Mikko Perttunen > --- > v7: > * Allocate gather BO with DMA API to get page-aligned > memory > * Add error prints to a few places where they were missing > v6: > * Remove sgt bypass path in gather_bo - this would cause > cache maintenance to be skipped and is unnecessary in > general. > * Changes related to moving to using syncpoint IDs > * Add syncobj related code > * Print warning on submit failure describing the issue > * Use ARCH_DMA_ADDR_T_64BIT to check if that is indeed > the case > * Add support for relative syncpoint wait > * Use pm_runtime_resume_and_get > * Only try to resume engines that support runtime PM > * Removed uapi subdirectory > * Don't use "copy_err" variables for copy_from_user > return value > * Fix setting of blocklinear flag > v5: > * Add 16K size limit to copies from userspace. > * Guard RELOC_BLOCKLINEAR flag handling to only exist in ARM64 > to prevent oversized shift on 32-bit platforms. > v4: > * Remove all features that are not strictly necessary. > * Split into two patches. > v3: > * Remove WRITE_RELOC. Relocations are now patched implicitly > when patching is needed. > * Directly call PM runtime APIs on devices instead of using > power_on/power_off callbacks. > * Remove incorrect mutex unlock in tegra_drm_ioctl_channel_open > * Use XA_FLAGS_ALLOC1 instead of XA_FLAGS_ALLOC > * Accommodate for removal of timeout field and inlining of > syncpt_incrs array. > * Copy entire user arrays at a time instead of going through > elements one-by-one. > * Implement waiting of DMA reservations. > * Split out gather_bo implementation into a separate file. > * Fix length parameter passed to sg_init_one in gather_bo > * Cosmetic cleanup. > --- > drivers/gpu/drm/tegra/Makefile| 2 + > drivers/gpu/drm/tegra/drm.c | 4 +- > drivers/gpu/drm/tegra/gather_bo.c | 82 + > drivers/gpu/drm/tegra/gather_bo.h | 24 ++ > drivers/gpu/drm/tegra/submit.c| 549 ++ > drivers/gpu/drm/tegra/submit.h| 17 + > 6 files changed, 677 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/tegra/gather_bo.c > create mode 100644 drivers/gpu/drm/tegra/gather_bo.h > create mode 100644 drivers/gpu/drm/tegra/submit.c > create mode 100644 drivers/gpu/drm/tegra/submit.h ... > +int tegra_drm_ioctl_channel_submit(struct drm_device *drm, void *data, > +struct drm_file *file) > +{ > + struct tegra_drm_file *fpriv = file->driver_priv; > + struct drm_tegra_channel_submit *args = data; > + struct tegra_drm_submit_data *job_data; > + struct drm_syncobj *syncobj = NULL; > + struct tegra_drm_context *ctx; > + struct host1x_job *job; > + struct gather_bo *bo; > + u32 i; > + int err; > + > + mutex_lock(>lock); > + ctx = xa_load(>contexts, args->channel_ctx); > + if (!ctx) { > + mutex_unlock(>lock); > + pr_err_ratelimited("%s: %s: invalid channel_ctx '%d'", __func__, > + current->comm, args->channel_ctx); > + return -EINVAL; > + } > + > + if (args->syncobj_in) { > + struct dma_fence *fence; > + > + err = drm_syncobj_find_fence(file, args->syncobj_in, 0, 0, > ); > + if (err) { > + SUBMIT_ERR(ctx, "invalid syncobj_in '%d'", > args->syncobj_in); > + goto unlock; > + } > + > + err = dma_fence_wait_timeout(fence, true, > msecs_to_jiffies(1)); > + dma_fence_put(fence); > + if (err) { > + SUBMIT_ERR(ctx, "wait for syncobj_in timed out"); > + goto unlock; > + } > + } > + > + if (args->syncobj_out) { > + syncobj = drm_syncobj_find(file, args->syncobj_out); > + if (!syncobj) { > + SUBMIT_ERR(ctx, "invalid syncobj_out '%d'", > args->syncobj_out); > + err = -ENOENT; > + goto unlock; > + } > + } > + > + /* Allocate gather BO and copy gather words in. */ > + err = submit_copy_gather_data(, drm->dev, ctx, args); > + if (err) > + goto unlock; > + > + job_data = kzalloc(sizeof(*job_data), GFP_KERNEL); > + if (!job_data) { > + SUBMIT_ERR(ctx, "failed to allocate memory for job data"); > + err = -ENOMEM; > + goto put_bo; > + } > + > + /* Get data buffer mappings and do relocation patching. */ > + err = submit_process_bufs(ctx, bo, args, job_data); > + if (err) > + goto free_job_data; > + > + /* Allocate host1x_job and add gathers and waits to it. */ > + err = submit_create_job(, ctx, bo, args, job_data, > + >syncpoints); > + if (err) > + goto free_job_data;
Re: [PATCH 2/2] drm/tegra: sor: Fully initialize SOR before registration
On 01/04/2021 16:41, Thierry Reding wrote: > From: Thierry Reding > > Before registering the SOR host1x client, make sure that it is fully > initialized. This avoids a potential race condition between the SOR's > probe and the host1x device initialization in cases where the SOR is > the final sub-device to register to a host1x instance. > > Reported-by: Jonathan Hunter > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/tegra/sor.c | 27 +-- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c > index 7b88261f57bb..b29bc10a0a4d 100644 > --- a/drivers/gpu/drm/tegra/sor.c > +++ b/drivers/gpu/drm/tegra/sor.c > @@ -3916,17 +3916,10 @@ static int tegra_sor_probe(struct platform_device > *pdev) > platform_set_drvdata(pdev, sor); > pm_runtime_enable(>dev); > > - INIT_LIST_HEAD(>client.list); > + host1x_client_init(>client); > sor->client.ops = _client_ops; > sor->client.dev = >dev; > > - err = host1x_client_register(>client); > - if (err < 0) { > - dev_err(>dev, "failed to register host1x client: %d\n", > - err); > - goto rpm_disable; > - } > - > /* >* On Tegra210 and earlier, provide our own implementation for the >* pad output clock. > @@ -3938,13 +3931,13 @@ static int tegra_sor_probe(struct platform_device > *pdev) > sor->index); > if (!name) { > err = -ENOMEM; > - goto unregister; > + goto uninit; > } > > err = host1x_client_resume(>client); > if (err < 0) { > dev_err(sor->dev, "failed to resume: %d\n", err); > - goto unregister; > + goto uninit; > } > > sor->clk_pad = tegra_clk_sor_pad_register(sor, name); > @@ -3955,14 +3948,20 @@ static int tegra_sor_probe(struct platform_device > *pdev) > err = PTR_ERR(sor->clk_pad); > dev_err(sor->dev, "failed to register SOR pad clock: %d\n", > err); > - goto unregister; > + goto uninit; > + } > + > + err = __host1x_client_register(>client); > + if (err < 0) { > + dev_err(>dev, "failed to register host1x client: %d\n", > + err); > + goto uninit; > } > > return 0; > > -unregister: > - host1x_client_unregister(>client); > -rpm_disable: > +uninit: > + host1x_client_exit(>client); > pm_runtime_disable(>dev); > remove: > tegra_output_remove(>output); > Thanks! Completed 200 boots on Jetson TX1 without any further probing issues, so ... Tested-by: Jon Hunter Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/tegra: Add NVDEC driver
On 13/02/2021 10:15, Mikko Perttunen wrote: > Add support for booting and using NVDEC on Tegra210, Tegra186 > and Tegra194 to the Host1x and TegraDRM drivers. Booting in > secure mode is not currently supported. > > Signed-off-by: Mikko Perttunen > --- > drivers/gpu/drm/tegra/Makefile | 3 +- > drivers/gpu/drm/tegra/drm.c| 4 + > drivers/gpu/drm/tegra/drm.h| 1 + > drivers/gpu/drm/tegra/nvdec.c | 497 + > drivers/gpu/host1x/dev.c | 12 + > include/linux/host1x.h | 1 + > 6 files changed, 517 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/tegra/nvdec.c ... > +static int nvdec_probe(struct platform_device *pdev) > +{ > + struct device *dev = >dev; > + struct host1x_syncpt **syncpts; > + struct resource *regs; > + struct nvdec *nvdec; > + int err; > + > + /* inherit DMA mask from host1x parent */ > + err = dma_coerce_mask_and_coherent(dev, *dev->parent->dma_mask); > + if (err < 0) { > + dev_err(>dev, "failed to set DMA mask: %d\n", err); > + return err; > + } > + > + nvdec = devm_kzalloc(dev, sizeof(*nvdec), GFP_KERNEL); > + if (!nvdec) > + return -ENOMEM; > + > + nvdec->config = of_device_get_match_data(dev); > + > + syncpts = devm_kzalloc(dev, sizeof(*syncpts), GFP_KERNEL); > + if (!syncpts) > + return -ENOMEM; > + > + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!regs) { > + dev_err(>dev, "failed to get registers\n"); > + return -ENXIO; > + } > + > + nvdec->regs = devm_ioremap_resource(dev, regs); > + if (IS_ERR(nvdec->regs)) > + return PTR_ERR(nvdec->regs); > + We should be able to use devm_platform_get_and_ioremap_resource() here. > + nvdec->clk = devm_clk_get(dev, NULL); > + if (IS_ERR(nvdec->clk)) { > + dev_err(>dev, "failed to get clock\n"); > + return PTR_ERR(nvdec->clk); > + } > + > + if (!dev->pm_domain) { Looks like the power-domain is required by device-tree and so do we need this? > + nvdec->rst = devm_reset_control_get(dev, "nvdec"); > + if (IS_ERR(nvdec->rst)) { > + dev_err(>dev, "failed to get reset\n"); > + return PTR_ERR(nvdec->rst); > + } > + } > + > + nvdec->falcon.dev = dev; > + nvdec->falcon.regs = nvdec->regs; > + > + err = falcon_init(>falcon); > + if (err < 0) > + return err; > + > + platform_set_drvdata(pdev, nvdec); > + > + INIT_LIST_HEAD(>client.base.list); > + nvdec->client.base.ops = _client_ops; > + nvdec->client.base.dev = dev; > + nvdec->client.base.class = HOST1X_CLASS_NVDEC; > + nvdec->client.base.syncpts = syncpts; > + nvdec->client.base.num_syncpts = 1; > + nvdec->dev = dev; > + > + INIT_LIST_HEAD(>client.list); > + nvdec->client.version = nvdec->config->version; > + nvdec->client.ops = _ops; > + > + err = host1x_client_register(>client.base); > + if (err < 0) { > + dev_err(dev, "failed to register host1x client: %d\n", err); > + goto exit_falcon; > + } > + > + pm_runtime_enable(>dev); > + if (!pm_runtime_enabled(>dev)) { > + err = nvdec_runtime_resume(>dev); > + if (err < 0) > + goto unregister_client; > + } pm_runtime should always be enabled for 64-bit Tegra and so we should not need to check pm_runtime_enabled(). Cheers Jon -- nvpublic ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/8] Host1x context isolation support
Will, Joerg, Rob, On 08/11/2021 10:36, Mikko Perttunen wrote: On 9/16/21 5:32 PM, Mikko Perttunen wrote: Hi all, *** New in v2: Added support for Tegra194 Use standard iommu-map property instead of custom mechanism *** this series adds support for Host1x 'context isolation'. Since when programming engines through Host1x, userspace can program in any addresses it wants, we need some way to isolate the engines' memory spaces. Traditionally this has either been done imperfectly with a single shared IOMMU domain, or by copying and verifying the programming command stream at submit time (Host1x firewall). Since Tegra186 there is a privileged (only usable by kernel) Host1x opcode that allows setting the stream ID sent by the engine to the SMMU. So, by allocating a number of context banks and stream IDs for this purpose, and using this opcode at the beginning of each job, we can implement isolation. Due to the limited number of context banks only each process gets its own context, and not each channel. This feature also allows sharing engines among multiple VMs when used with Host1x's hardware virtualization support - up to 8 VMs can be configured with a subset of allowed stream IDs, enforced at hardware level. To implement this, this series adds a new host1x context bus, which will contain the 'struct device's corresponding to each context bank / stream ID, changes to device tree and SMMU code to allow registering the devices and using the bus, as well as the Host1x stream ID programming code and support in TegraDRM. Device tree bindings are not updated yet pending consensus that the proposed changes make sense. Thanks, Mikko Mikko Perttunen (8): gpu: host1x: Add context bus gpu: host1x: Add context device management code gpu: host1x: Program context stream ID on submission iommu/arm-smmu: Attach to host1x context device bus arm64: tegra: Add Host1x context stream IDs on Tegra186+ drm/tegra: falcon: Set DMACTX field on DMA transactions drm/tegra: vic: Implement get_streamid_offset drm/tegra: Support context isolation arch/arm64/boot/dts/nvidia/tegra186.dtsi | 12 ++ arch/arm64/boot/dts/nvidia/tegra194.dtsi | 12 ++ drivers/gpu/Makefile | 3 +- drivers/gpu/drm/tegra/drm.h | 2 + drivers/gpu/drm/tegra/falcon.c | 8 + drivers/gpu/drm/tegra/falcon.h | 1 + drivers/gpu/drm/tegra/submit.c | 13 ++ drivers/gpu/drm/tegra/uapi.c | 34 - drivers/gpu/drm/tegra/vic.c | 38 + drivers/gpu/host1x/Kconfig | 5 + drivers/gpu/host1x/Makefile | 2 + drivers/gpu/host1x/context.c | 174 ++ drivers/gpu/host1x/context.h | 27 drivers/gpu/host1x/context_bus.c | 31 drivers/gpu/host1x/dev.c | 12 +- drivers/gpu/host1x/dev.h | 2 + drivers/gpu/host1x/hw/channel_hw.c | 52 ++- drivers/gpu/host1x/hw/host1x06_hardware.h | 10 ++ drivers/gpu/host1x/hw/host1x07_hardware.h | 10 ++ drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 ++ include/linux/host1x.h | 21 +++ include/linux/host1x_context_bus.h | 15 ++ 22 files changed, 488 insertions(+), 9 deletions(-) create mode 100644 drivers/gpu/host1x/context.c create mode 100644 drivers/gpu/host1x/context.h create mode 100644 drivers/gpu/host1x/context_bus.c create mode 100644 include/linux/host1x_context_bus.h IOMMU/DT folks, any thoughts about this approach? The patches that are of interest outside of Host1x/TegraDRM specifics are patches 1, 2, 4, and 5. Any feedback on this? Jon -- nvpublic
Re: [PATCH v2 0/8] Host1x context isolation support
On 14/12/2021 15:38, Robin Murphy wrote: ... IOMMU/DT folks, any thoughts about this approach? The patches that are of interest outside of Host1x/TegraDRM specifics are patches 1, 2, 4, and 5. FWIW it looks fairly innocuous to me. I don't understand host1x - neither hardware nor driver abstractions - well enough to meaningfully review it all (e.g. maybe it's deliberate that the bus .dma_configure method isn't used?), but the SMMU patch seems fine given the Kconfig solution to avoid module linkage problems. Thanks Robin! Will, Joerg, is OK with you? Mikko, I believe we are missing a dt-binding change to document the 'memory-contexts' node which I assume you will add if everyone is OK with this? Cheers Jon -- nvpublic
Re: [PATCH v16 08/40] gpu: host1x: Add initial runtime PM and OPP support
On 22/12/2021 19:01, Dmitry Osipenko wrote: ... diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c index e08e331e46ae..8194826c9ce3 100644 --- a/drivers/gpu/host1x/syncpt.c +++ b/drivers/gpu/host1x/syncpt.c @@ -137,6 +137,15 @@ void host1x_syncpt_restore(struct host1x *host) struct host1x_syncpt *sp_base = host->syncpt; unsigned int i; + for (i = 0; i < host->info->nb_pts; i++) { + /* +* Unassign syncpt from channels for purposes of Tegra186 +* syncpoint protection. This prevents any channel from +* accessing it until it is reassigned. +*/ + host1x_hw_syncpt_assign_to_channel(host, sp_base + i, NULL); + } + for (i = 0; i < host1x_syncpt_nb_pts(host); i++) host1x_hw_syncpt_restore(host, sp_base + i); @@ -352,13 +361,6 @@ int host1x_syncpt_init(struct host1x *host) for (i = 0; i < host->info->nb_pts; i++) { syncpt[i].id = i; syncpt[i].host = host; - - /* -* Unassign syncpt from channels for purposes of Tegra186 -* syncpoint protection. This prevents any channel from -* accessing it until it is reassigned. -*/ - host1x_hw_syncpt_assign_to_channel(host, [i], NULL); } for (i = 0; i < host->info->nb_bases; i++) Thanks! This fixed it! Jon -- nvpublic
Re: [PATCH v16 08/40] gpu: host1x: Add initial runtime PM and OPP support
On 22/12/2021 09:47, Jon Hunter wrote: On 21/12/2021 20:58, Dmitry Osipenko wrote: Hi, Thank you for testing it all. 21.12.2021 21:55, Jon Hunter пишет: Hi Dmitry, Thierry, On 30/11/2021 23:23, Dmitry Osipenko wrote: Add runtime PM and OPP support to the Host1x driver. For the starter we will keep host1x always-on because dynamic power management require a major refactoring of the driver code since lot's of code paths are missing the RPM handling and we're going to remove some of these paths in the future. Unfortunately, this change is breaking boot on Tegra186. Bisect points to this and reverting on top of -next gets the board booting again. Sadly, there is no panic or error reported, it is just a hard hang. I will not have time to look at this this week and so we may need to revert for the moment. Only T186 broken? What about T194? Yes interestingly only Tegra186 and no other board. Which board model fails to boot? Is it running in hypervisor mode? This is Jetson TX2. No hypervisor. Do you use any additional patches? No just plain -next. The tests run every day on top of tree. Could you please test the below diff? I suspect that host1x_syncpt_save/restore may be entirely broken for T186 since we never used these funcs before. --- >8 --- diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index f5b4dcded088..fd5dfb875422 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -580,7 +580,6 @@ static int __maybe_unused host1x_runtime_suspend(struct device *dev) int err; host1x_intr_stop(host); - host1x_syncpt_save(host); err = reset_control_bulk_assert(host->nresets, host->resets); if (err) { @@ -596,9 +595,8 @@ static int __maybe_unused host1x_runtime_suspend(struct device *dev) return 0; resume_host1x: - host1x_setup_sid_table(host); - host1x_syncpt_restore(host); host1x_intr_start(host); + host1x_setup_sid_table(host); return err; } @@ -626,9 +624,8 @@ static int __maybe_unused host1x_runtime_resume(struct device *dev) goto disable_clk; } - host1x_setup_sid_table(host); - host1x_syncpt_restore(host); host1x_intr_start(host); + host1x_setup_sid_table(host); Thanks! Will try this later, once the next bisect is finished :-) I tested the above, but this did not fix it. It still hangs on boot. Jon -- nvpublic
Re: [PATCH v16 08/40] gpu: host1x: Add initial runtime PM and OPP support
Hi Dmitry, Thierry, On 30/11/2021 23:23, Dmitry Osipenko wrote: Add runtime PM and OPP support to the Host1x driver. For the starter we will keep host1x always-on because dynamic power management require a major refactoring of the driver code since lot's of code paths are missing the RPM handling and we're going to remove some of these paths in the future. Unfortunately, this change is breaking boot on Tegra186. Bisect points to this and reverting on top of -next gets the board booting again. Sadly, there is no panic or error reported, it is just a hard hang. I will not have time to look at this this week and so we may need to revert for the moment. Jon -- nvpublic
Re: [PATCH v16 08/40] gpu: host1x: Add initial runtime PM and OPP support
On 21/12/2021 20:58, Dmitry Osipenko wrote: Hi, Thank you for testing it all. 21.12.2021 21:55, Jon Hunter пишет: Hi Dmitry, Thierry, On 30/11/2021 23:23, Dmitry Osipenko wrote: Add runtime PM and OPP support to the Host1x driver. For the starter we will keep host1x always-on because dynamic power management require a major refactoring of the driver code since lot's of code paths are missing the RPM handling and we're going to remove some of these paths in the future. Unfortunately, this change is breaking boot on Tegra186. Bisect points to this and reverting on top of -next gets the board booting again. Sadly, there is no panic or error reported, it is just a hard hang. I will not have time to look at this this week and so we may need to revert for the moment. Only T186 broken? What about T194? Yes interestingly only Tegra186 and no other board. Which board model fails to boot? Is it running in hypervisor mode? This is Jetson TX2. No hypervisor. Do you use any additional patches? No just plain -next. The tests run every day on top of tree. Could you please test the below diff? I suspect that host1x_syncpt_save/restore may be entirely broken for T186 since we never used these funcs before. --- >8 --- diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index f5b4dcded088..fd5dfb875422 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -580,7 +580,6 @@ static int __maybe_unused host1x_runtime_suspend(struct device *dev) int err; host1x_intr_stop(host); - host1x_syncpt_save(host); err = reset_control_bulk_assert(host->nresets, host->resets); if (err) { @@ -596,9 +595,8 @@ static int __maybe_unused host1x_runtime_suspend(struct device *dev) return 0; resume_host1x: - host1x_setup_sid_table(host); - host1x_syncpt_restore(host); host1x_intr_start(host); + host1x_setup_sid_table(host); return err; } @@ -626,9 +624,8 @@ static int __maybe_unused host1x_runtime_resume(struct device *dev) goto disable_clk; } - host1x_setup_sid_table(host); - host1x_syncpt_restore(host); host1x_intr_start(host); + host1x_setup_sid_table(host); Thanks! Will try this later, once the next bisect is finished :-) Jon -- nvpublic
Re: [PATCH v2 0/8] Host1x context isolation support
Hi all, Still no response on this :-( On 06/12/2021 09:55, Jon Hunter wrote: Will, Joerg, Rob, On 08/11/2021 10:36, Mikko Perttunen wrote: On 9/16/21 5:32 PM, Mikko Perttunen wrote: Hi all, *** New in v2: Added support for Tegra194 Use standard iommu-map property instead of custom mechanism *** this series adds support for Host1x 'context isolation'. Since when programming engines through Host1x, userspace can program in any addresses it wants, we need some way to isolate the engines' memory spaces. Traditionally this has either been done imperfectly with a single shared IOMMU domain, or by copying and verifying the programming command stream at submit time (Host1x firewall). Since Tegra186 there is a privileged (only usable by kernel) Host1x opcode that allows setting the stream ID sent by the engine to the SMMU. So, by allocating a number of context banks and stream IDs for this purpose, and using this opcode at the beginning of each job, we can implement isolation. Due to the limited number of context banks only each process gets its own context, and not each channel. This feature also allows sharing engines among multiple VMs when used with Host1x's hardware virtualization support - up to 8 VMs can be configured with a subset of allowed stream IDs, enforced at hardware level. To implement this, this series adds a new host1x context bus, which will contain the 'struct device's corresponding to each context bank / stream ID, changes to device tree and SMMU code to allow registering the devices and using the bus, as well as the Host1x stream ID programming code and support in TegraDRM. Device tree bindings are not updated yet pending consensus that the proposed changes make sense. Thanks, Mikko Mikko Perttunen (8): gpu: host1x: Add context bus gpu: host1x: Add context device management code gpu: host1x: Program context stream ID on submission iommu/arm-smmu: Attach to host1x context device bus arm64: tegra: Add Host1x context stream IDs on Tegra186+ drm/tegra: falcon: Set DMACTX field on DMA transactions drm/tegra: vic: Implement get_streamid_offset drm/tegra: Support context isolation arch/arm64/boot/dts/nvidia/tegra186.dtsi | 12 ++ arch/arm64/boot/dts/nvidia/tegra194.dtsi | 12 ++ drivers/gpu/Makefile | 3 +- drivers/gpu/drm/tegra/drm.h | 2 + drivers/gpu/drm/tegra/falcon.c | 8 + drivers/gpu/drm/tegra/falcon.h | 1 + drivers/gpu/drm/tegra/submit.c | 13 ++ drivers/gpu/drm/tegra/uapi.c | 34 - drivers/gpu/drm/tegra/vic.c | 38 + drivers/gpu/host1x/Kconfig | 5 + drivers/gpu/host1x/Makefile | 2 + drivers/gpu/host1x/context.c | 174 ++ drivers/gpu/host1x/context.h | 27 drivers/gpu/host1x/context_bus.c | 31 drivers/gpu/host1x/dev.c | 12 +- drivers/gpu/host1x/dev.h | 2 + drivers/gpu/host1x/hw/channel_hw.c | 52 ++- drivers/gpu/host1x/hw/host1x06_hardware.h | 10 ++ drivers/gpu/host1x/hw/host1x07_hardware.h | 10 ++ drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 ++ include/linux/host1x.h | 21 +++ include/linux/host1x_context_bus.h | 15 ++ 22 files changed, 488 insertions(+), 9 deletions(-) create mode 100644 drivers/gpu/host1x/context.c create mode 100644 drivers/gpu/host1x/context.h create mode 100644 drivers/gpu/host1x/context_bus.c create mode 100644 include/linux/host1x_context_bus.h IOMMU/DT folks, any thoughts about this approach? The patches that are of interest outside of Host1x/TegraDRM specifics are patches 1, 2, 4, and 5. Any feedback on this? Jon -- nvpublic
[PATCH] drm/tegra: Fix compilation of variadic macro
Commit 43636451db8c ("drm/tegra: Implement job submission part of new UAPI") added the macro 'SUBMIT_ERR' that in turns makes use of the macro '__VA_OPT__'. The '__VA_OPT__' macro is not supported by older versions of GCC and so causes build failures when using older versions of GCC. Fix this by using the '##__VA_ARGS__' macro instead. Fixes: 43636451db8c ("drm/tegra: Implement job submission part of new UAPI") Reported-by: Linux Kernel Functional Testing Signed-off-by: Jon Hunter --- drivers/gpu/drm/tegra/submit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/submit.c b/drivers/gpu/drm/tegra/submit.c index c53b7207c478..e49630089149 100644 --- a/drivers/gpu/drm/tegra/submit.c +++ b/drivers/gpu/drm/tegra/submit.c @@ -24,7 +24,7 @@ #define SUBMIT_ERR(context, fmt, ...) \ dev_err_ratelimited(context->client->base.dev, \ "%s: job submission failed: " fmt "\n", \ - current->comm __VA_OPT__(,) __VA_ARGS__) + current->comm, ##__VA_ARGS__) static struct tegra_drm_mapping * tegra_drm_mapping_get(struct tegra_drm_context *context, u32 id) -- 2.25.1
[PATCH] drm/tegra: Fix cast to restricted __le32
Sparse warns about the following cast in the function falcon_copy_firmware_image() ... drivers/gpu/drm/tegra/falcon.c:66:27: warning: cast to restricted __le32 Fix this by casting the firmware data array to __le32 instead of u32. Signed-off-by: Jon Hunter --- drivers/gpu/drm/tegra/falcon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/falcon.c b/drivers/gpu/drm/tegra/falcon.c index 223ab2ceb7e6..3762d87759d9 100644 --- a/drivers/gpu/drm/tegra/falcon.c +++ b/drivers/gpu/drm/tegra/falcon.c @@ -63,7 +63,7 @@ static void falcon_copy_firmware_image(struct falcon *falcon, /* copy the whole thing taking into account endianness */ for (i = 0; i < firmware->size / sizeof(u32); i++) - virt[i] = le32_to_cpu(((u32 *)firmware->data)[i]); + virt[i] = le32_to_cpu(((__le32 *)firmware->data)[i]); } static int falcon_parse_firmware_image(struct falcon *falcon) -- 2.25.1
Re: [PATCH] gpu: host1x: Do not use mapping cache for job submissions
On 24/03/2022 10:30, Thierry Reding wrote: From: Thierry Reding Buffer mappings used in job submissions are usually small and not rapidly reused as opposed to framebuffers (which are usually large and rapidly reused, for example when page-flipping between double-buffered framebuffers). Avoid going through the mapping cache for these buffers since the cache would also lead to leaks if nobody is ever releasing the cache's last reference. For DRM/KMS these last references are dropped when the framebuffers are removed and therefore no longer needed. While at it, also add a note about the need to explicitly remove the final reference to the mapping in the cache. Signed-off-by: Thierry Reding I have tested this and verified that it is working well. Reviewed-by: Jon Hunter Tested-by: Jon Hunter Thanks Jon -- nvpublic
Re: [PATCH] drm: tegra: fix memory leak in error handling path
On 29/03/2022 11:37, cgel@gmail.com wrote: From: Lv Ruyi Before leave the nvdec_load_firmware, we shuold free virt which is alloced s/shuold/should s/alloced/allocated by dma_alloc_coherent, so change "return err" to "goto cleanup". Reported-by: Zeal Robot Signed-off-by: Lv Ruyi --- drivers/gpu/drm/tegra/nvdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tegra/nvdec.c b/drivers/gpu/drm/tegra/nvdec.c index 79e1e88203cf..a14863346bfa 100644 --- a/drivers/gpu/drm/tegra/nvdec.c +++ b/drivers/gpu/drm/tegra/nvdec.c @@ -209,7 +209,7 @@ static int nvdec_load_firmware(struct nvdec *nvdec) err = dma_mapping_error(nvdec->dev, iova); if (err < 0) - return err; + goto cleanup; Actually, I think that the correct fix here would be the same as what was done for VIC ... https://lore.kernel.org/linux-mm/6b86f6e530b504a5eee864af10e2ae1570d7b645.1639157090.git.robin.mur...@arm.com/ Jon -- nvpublic
[PATCH] gpu: host1x: Show all allocated syncpts via debugfs
When the host1x syncpts status is dumped via the debugfs, syncpts that have been allocated but not yet used are not shown and so currently it is not possible to see all the allocated syncpts. Update the path for dumping the syncpt status via the debugfs to show all allocated syncpts even if they have not been used yet. Note that when the syncpt status is dumped by the kernel itself for debugging only the active syncpt are shown. Signed-off-by: Jon Hunter --- drivers/gpu/host1x/debug.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/host1x/debug.c b/drivers/gpu/host1x/debug.c index 18d9c8d206e3..34c2e36d09e9 100644 --- a/drivers/gpu/host1x/debug.c +++ b/drivers/gpu/host1x/debug.c @@ -75,7 +75,7 @@ static int show_channel(struct host1x_channel *ch, void *data, bool show_fifo) return 0; } -static void show_syncpts(struct host1x *m, struct output *o) +static void show_syncpts(struct host1x *m, struct output *o, bool show_all) { struct list_head *pos; unsigned int i; @@ -97,7 +97,10 @@ static void show_syncpts(struct host1x *m, struct output *o) waiters++; spin_unlock(>syncpt[i].intr.lock); - if (!min && !max && !waiters) + if (!kref_read(>syncpt[i].ref)) + continue; + + if (!show_all && !min && !max && !waiters) continue; host1x_debug_output(o, @@ -124,7 +127,7 @@ static void show_all(struct host1x *m, struct output *o, bool show_fifo) unsigned int i; host1x_hw_show_mlocks(m, o); - show_syncpts(m, o); + show_syncpts(m, o, true); host1x_debug_output(o, " channels \n"); for (i = 0; i < m->info->nb_channels; ++i) { @@ -241,5 +244,5 @@ void host1x_debug_dump_syncpts(struct host1x *host1x) .fn = write_to_printk }; - show_syncpts(host1x, ); + show_syncpts(host1x, , false); } -- 2.25.1
Re: [PATCH v8 01/16] clk: generalize devm_clk_get() a bit
Hi Uwe, On 14/03/2022 14:16, Uwe Kleine-König wrote: Allow to add an exit hook to devm managed clocks. Also use clk_get_optional() in devm_clk_get_optional instead of open coding it. The generalisation will be used in the next commit to add some more devm_clk helpers. Reviewed-by: Jonathan Cameron Reviewed-by: Alexandru Ardelean Signed-off-by: Uwe Kleine-König --- drivers/clk/clk-devres.c | 67 ++-- 1 file changed, 50 insertions(+), 17 deletions(-) diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c index f9d5b7334341..fb7761888b30 100644 --- a/drivers/clk/clk-devres.c +++ b/drivers/clk/clk-devres.c @@ -4,39 +4,72 @@ #include #include +struct devm_clk_state { + struct clk *clk; + void (*exit)(struct clk *clk); +}; + static void devm_clk_release(struct device *dev, void *res) { - clk_put(*(struct clk **)res); + struct devm_clk_state *state = *(struct devm_clk_state **)res; + + if (state->exit) + state->exit(state->clk); + + clk_put(state->clk); } -struct clk *devm_clk_get(struct device *dev, const char *id) +static struct clk *__devm_clk_get(struct device *dev, const char *id, + struct clk *(*get)(struct device *dev, const char *id), + int (*init)(struct clk *clk), + void (*exit)(struct clk *clk)) { - struct clk **ptr, *clk; + struct devm_clk_state *state; + struct clk *clk; + int ret; - ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL); - if (!ptr) + state = devres_alloc(devm_clk_release, sizeof(*state), GFP_KERNEL); + if (!state) return ERR_PTR(-ENOMEM); - clk = clk_get(dev, id); - if (!IS_ERR(clk)) { - *ptr = clk; - devres_add(dev, ptr); - } else { - devres_free(ptr); + clk = get(dev, id); + if (IS_ERR(clk)) { + ret = PTR_ERR(clk); + goto err_clk_get; } + if (init) { + ret = init(clk); + if (ret) + goto err_clk_init; + } + + state->clk = clk; + state->exit = exit; + + devres_add(dev, state); + return clk; + +err_clk_init: + + clk_put(clk); +err_clk_get: + + devres_free(state); + return ERR_PTR(ret); } -EXPORT_SYMBOL(devm_clk_get); -struct clk *devm_clk_get_optional(struct device *dev, const char *id) +struct clk *devm_clk_get(struct device *dev, const char *id) { - struct clk *clk = devm_clk_get(dev, id); + return __devm_clk_get(dev, id, clk_get, NULL, NULL); - if (clk == ERR_PTR(-ENOENT)) - return NULL; +} +EXPORT_SYMBOL(devm_clk_get); - return clk; +struct clk *devm_clk_get_optional(struct device *dev, const char *id) +{ + return __devm_clk_get(dev, id, clk_get_optional, NULL, NULL); } EXPORT_SYMBOL(devm_clk_get_optional); Some of our Tegra boards are not booting with the current -next and bisect is pointing to this commit. Looking at the boot log I am seeing the following panic ... [2.097048] 8<--- cut here --- [2.097053] Unable to handle kernel paging request at virtual address c216c810 [2.097060] [c216c810] *pgd=0201141e(bad) [2.097079] Internal error: Oops: 800d [#1] SMP ARM [2.097088] Modules linked in: [2.097097] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc3-next-20220621-g34d1d36073ea #1 [2.097107] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [2.097113] PC is at 0xc216c810 [2.097123] LR is at devm_clk_release+0x18/0x24 [2.097150] pc : []lr : []psr: a013 [2.097155] sp : f080dde8 ip : 06cf fp : c18d4854 [2.097161] r10: c1501850 r9 : c1a04d10 r8 : c1c4efa0 [2.097166] r7 : c216c810 r6 : f080de1c r5 : c2737680 r4 : c26a9680 [2.097172] r3 : c216c810 r2 : r1 : c2737840 r0 : c2082840 [2.097179] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [2.097187] Control: 10c5387d Table: 0020404a DAC: 0051 [2.097191] Register r0 information: slab kmalloc-192 start c2082840 pointer offset 0 size 192 [2.097216] Register r1 information: slab kmalloc-128 start c2737800 pointer offset 64 size 128 [2.097236] Register r2 information: NULL pointer [2.097244] Register r3 information: slab kmalloc-1k start c216c800 pointer offset 16 size 1024 [2.097263] Register r4 information: slab kmalloc-64 start c26a9680 pointer offset 0 size 64 [2.097282] Register r5 information: slab kmalloc-128 start c2737680 pointer offset 0 size 128 [2.097301] Register r6 information: 2-page vmalloc region starting at 0xf080c000 allocated at kernel_clone+0xb4/0x3e8 [2.097321] Register r7 information: slab kmalloc-1k start c216c800 pointer offset 16 size 1024 [2.097341] Register r8 information:
Re: [PATCH v8 01/16] clk: generalize devm_clk_get() a bit
On 21/06/2022 21:49, Uwe Kleine-König wrote: On Tue, Jun 21, 2022 at 08:57:00PM +0100, Jon Hunter wrote: Some of our Tegra boards are not booting with the current -next and bisect is pointing to this commit. Looking at the boot log I am seeing the following panic ... [2.097048] 8<--- cut here --- [2.097053] Unable to handle kernel paging request at virtual address c216c810 [2.097060] [c216c810] *pgd=0201141e(bad) [2.097079] Internal error: Oops: 800d [#1] SMP ARM [2.097088] Modules linked in: [2.097097] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc3-next-20220621-g34d1d36073ea #1 [2.097107] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [2.097113] PC is at 0xc216c810 [2.097123] LR is at devm_clk_release+0x18/0x24 [2.097150] pc : []lr : []psr: a013 [2.097155] sp : f080dde8 ip : 06cf fp : c18d4854 [2.097161] r10: c1501850 r9 : c1a04d10 r8 : c1c4efa0 [2.097166] r7 : c216c810 r6 : f080de1c r5 : c2737680 r4 : c26a9680 [2.097172] r3 : c216c810 r2 : r1 : c2737840 r0 : c2082840 [2.097179] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [2.097187] Control: 10c5387d Table: 0020404a DAC: 0051 [2.097191] Register r0 information: slab kmalloc-192 start c2082840 pointer offset 0 size 192 [2.097216] Register r1 information: slab kmalloc-128 start c2737800 pointer offset 64 size 128 [2.097236] Register r2 information: NULL pointer [2.097244] Register r3 information: slab kmalloc-1k start c216c800 pointer offset 16 size 1024 [2.097263] Register r4 information: slab kmalloc-64 start c26a9680 pointer offset 0 size 64 [2.097282] Register r5 information: slab kmalloc-128 start c2737680 pointer offset 0 size 128 [2.097301] Register r6 information: 2-page vmalloc region starting at 0xf080c000 allocated at kernel_clone+0xb4/0x3e8 [2.097321] Register r7 information: slab kmalloc-1k start c216c800 pointer offset 16 size 1024 [2.097341] Register r8 information: non-slab/vmalloc memory [2.097348] Register r9 information: non-slab/vmalloc memory [2.097355] Register r10 information: non-slab/vmalloc memory [2.097362] Register r11 information: non-slab/vmalloc memory [2.097369] Register r12 information: non-paged memory [2.097375] Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) [2.097384] Stack: (0xf080dde8 to 0xf080e000) [2.097394] dde0: c2737800 c0a72d38 c18d4854 c0530490 c216c810 f080de1c [2.097404] de00: c212 0005 c216c9c0 8013 017e c0a73d68 0008 c2629e00 [2.097413] de20: c2737880 5640e141 c216c810 c216c810 0205 c1c09dd4 c27375b8 [2.097422] de40: c2091700 c0a6e9a0 c216c810 c0a6f288 c216c810 c1c09dd4 c216c810 [2.097430] de60: c27375b8 c0a6f3c0 c1caa8e0 c216c810 c216c810 c0a6f450 c216c810 [2.097439] de80: c1c09dd4 c212 c27375b8 c0a6f850 c1c09dd4 c0a6f7c4 c0a6d4c0 [2.097447] dea0: c2091458 c2286434 5640e141 c1be7f08 c1c09dd4 c2737580 c1be7f08 [2.097455] dec0: c0a6e484 c1615714 c1be7c50 c1c09dd4 c212 c189a99c [2.097464] dee0: c212 c0a701a0 c1c494e0 c212 c189a99c c0302144 017d c0364438 [2.097472] df00: c16da8bc c1626700 0006 0006 c16554c8 c212 [2.097480] df20: c15105bc c14f9778 c2091700 c20917d9 5640e141 c1a88930 c16da8bc [2.097488] df40: c1c59000 5640e141 c16da8bc c1c59000 c1953b4c c18d4834 0007 c1801340 [2.097497] df60: 0006 0006 c18004dc c212 c18004dc f080df74 c1a04cc0 [2.097505] df80: c106bbf0 c106bc08 [2.097513] dfa0: c106bbf0 c03001a8 [2.097520] dfc0: [2.097528] dfe0: 0013 [2.097542] devm_clk_release from release_nodes+0x58/0xc0 [2.097575] release_nodes from devres_release_all+0x7c/0xc0 [2.097596] devres_release_all from device_unbind_cleanup+0xc/0x60 [2.097626] device_unbind_cleanup from really_probe+0x1f4/0x2a8 [2.097650] really_probe from __driver_probe_device+0x84/0xe4 [2.097673] __driver_probe_device from driver_probe_device+0x30/0xd0 [2.097696] driver_probe_device from __driver_attach+0x8c/0xf0 [2.097713] __driver_attach from bus_for_each_dev+0x70/0xb0 [2.097729] bus_for_each_dev from bus_add_driver+0x168/0x1f4 [2.097749] bus_add_driver from driver_register+0x7c/0x118 [2.097766] driver_register from do_one_initcall+0x44/0x1ec [2.097784] do_one_initcall from kernel_init_freeable+0x1d4/0x224 [2.097803] kernel_init_freeable from kernel_init+0x18/0x12c [2.097820] kernel_init from ret_from_fork+0x14/0x2c [2.097831] Except
Re: [PATCH v2 0/4] drm/nvdla: Add driver support for NVDLA
On 28/04/2022 16:56, Mikko Perttunen wrote: On 4/28/22 17:10, Thierry Reding wrote: On Tue, Apr 26, 2022 at 02:07:57PM +0800, Cai Huoqing wrote: The NVIDIA Deep Learning Accelerator (NVDLA) is an open source IP which is integrated into NVIDIA Jetson AGX Xavier, so add driver support for this accelerator." Hi, nice to see this work going on. For subsequent revisions, can you please also Cc the Tegra mailing list (linux-te...@vger.kernel.org) as well as the Tegra platform maintainers (that's Jon Hunter and myself). This will make sure that more people with an interest in this will see your work. Not everyone follows dri-devel, linaro-mm-sig or linux-media. Thanks, Thierry From a quick glance it looks like this driver pokes DLA hardware directly which is not the intended programming model on Tegra hardware (there are Falcon microcontrollers that offload task scheduling and synchronization from the CPU). The hardware is also behind the Host1x bus so a simple platform device is not sufficient. Was this driver developed against some platform with OpenDLA hardware (i.e. not Tegra)? If so, we'd need to verify if the hardware matches the hardware in Tegra194. Also, this driver may not be ideal for Tegra platforms since we would lack the hardware scheduling and synchronization facilities. It is likely necessary to have separate drivers for OpenDLA and Tegra's DLA integration. I believe that this is derived from the following github project ... https://github.com/nvdla/sw Jon -- nvpublic
Re: [PATCH v2] gpu: host1x: Avoid trying to use GART on Tegra20
On 20/10/2022 15:23, Robin Murphy wrote: Since commit c7e3ca515e78 ("iommu/tegra: gart: Do not register with bus") quite some time ago, the GART driver has effectively disabled itself to avoid issues with the GPU driver expecting it to work in ways that it doesn't. As of commit 57365a04c921 ("iommu: Move bus setup to IOMMU device registration") that bodge no longer works, but really the GPU driver should be responsible for its own behaviour anyway. Make the workaround explicit. Reported-by: Jon Hunter Suggested-by: Dmitry Osipenko Signed-off-by: Robin Murphy --- v2: Cover DRM instance too, move into *_wants_iommu() for consistency drivers/gpu/drm/tegra/drm.c | 4 drivers/gpu/host1x/dev.c| 4 2 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 6748ec1e0005..a1f909dac89a 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -1093,6 +1093,10 @@ static bool host1x_drm_wants_iommu(struct host1x_device *dev) struct host1x *host1x = dev_get_drvdata(dev->dev.parent); struct iommu_domain *domain; + /* Our IOMMU usage policy doesn't currently play well with GART */ + if (of_machine_is_compatible("nvidia,tegra20")) + return false; + /* * If the Tegra DRM clients are backed by an IOMMU, push buffers are * likely to be allocated beyond the 32-bit boundary if sufficient diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index 0cd3f97e7e49..f60ea24db0ec 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -292,6 +292,10 @@ static void host1x_setup_virtualization_tables(struct host1x *host) static bool host1x_wants_iommu(struct host1x *host1x) { + /* Our IOMMU usage policy doesn't currently play well with GART */ + if (of_machine_is_compatible("nvidia,tegra20")) + return false; + /* * If we support addressing a maximum of 32 bits of physical memory * and if the host1x firewall is enabled, there's no need to enable Thanks! Works for me. Tested-by: Jon Hunter Cheers Jon -- nvpublic
Re: [PATCH] gpu: host1x: Avoid trying to use GART on Tegra20
Hi Robin, On 19/10/2022 18:23, Robin Murphy wrote: Since commit c7e3ca515e78 ("iommu/tegra: gart: Do not register with bus") quite some time ago, the GART driver has effectively disabled itself to avoid issues with the GPU driver expecting it to work in ways that it doesn't. As of commit 57365a04c921 ("iommu: Move bus setup to IOMMU device registration") that bodge no longer works, but really the GPU driver should be responsible for its own behaviour anyway. Make the workaround explicit. Reported-by: Jon Hunter Suggested-by: Dmitry Osipenko Signed-off-by: Robin Murphy --- drivers/gpu/host1x/dev.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index a13fd9441edc..1cae8eea92cf 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -352,6 +352,10 @@ static struct iommu_domain *host1x_iommu_attach(struct host1x *host) if (!host1x_wants_iommu(host) || domain) return domain; + /* Our IOMMU usage policy doesn't currently play well with GART */ + if (of_machine_is_compatible("nvidia,tegra20")) + return NULL; + host->group = iommu_group_get(host->dev); if (host->group) { struct iommu_domain_geometry *geometry; Thanks for sending. I gave this a quick test, but I still see ... [2.901739] tegra-gr2d 5414.gr2d: failed to attach to domain: -19 [2.908373] drm drm: failed to initialize 5414.gr2d: -19 Cheers Jon -- nvpublic
Re: [PATCH v3 1/8] memory: tegra: Add API for retrieving carveout bounds
On 04/11/2022 15:35, Krzysztof Kozlowski wrote: On 04/11/2022 11:33, Jon Hunter wrote: Hi Thierry, Krzysztof, On 24/10/2022 14:15, Thierry Reding wrote: On Tue, Sep 20, 2022 at 11:11:56AM +0300, Mikko Perttunen wrote: From: Mikko Perttunen On Tegra234 NVDEC firmware is loaded from a secure carveout, where it has been loaded by a bootloader. When booting NVDEC, we need to tell it the address of this firmware, which we can determine by checking the starting address of the carveout. As such, add an MC API to query the bounds of carveouts, and add related information on Tegra234. Signed-off-by: Mikko Perttunen --- v2: - Add check for 64-bit phys_addr_t. In practice phys_addr_t is always 64 bits where this runs, but it avoids warnings in compile test. --- drivers/memory/tegra/mc.c | 25 + drivers/memory/tegra/tegra234.c | 5 + include/soc/tegra/mc.h | 11 +++ 3 files changed, 41 insertions(+) Krzysztof, I've applied this to the same tree as the patch that uses it for now. Let me know if you want me to put this on a separate stable branch for you to pull in. Any update on this? What kind of update do you expect? Ha! I guess I should be more explicit :-) Well, I would like to see this change in -next and so I was hoping that you would respond to the above to indicate how you would like to pull this in. Cheers! Jon -- nvpublic
Re: [PATCH v3 1/8] memory: tegra: Add API for retrieving carveout bounds
On 04/11/2022 15:48, Krzysztof Kozlowski wrote: On 04/11/2022 11:46, Jon Hunter wrote: On 04/11/2022 15:35, Krzysztof Kozlowski wrote: On 04/11/2022 11:33, Jon Hunter wrote: Hi Thierry, Krzysztof, On 24/10/2022 14:15, Thierry Reding wrote: On Tue, Sep 20, 2022 at 11:11:56AM +0300, Mikko Perttunen wrote: From: Mikko Perttunen On Tegra234 NVDEC firmware is loaded from a secure carveout, where it has been loaded by a bootloader. When booting NVDEC, we need to tell it the address of this firmware, which we can determine by checking the starting address of the carveout. As such, add an MC API to query the bounds of carveouts, and add related information on Tegra234. Signed-off-by: Mikko Perttunen --- v2: - Add check for 64-bit phys_addr_t. In practice phys_addr_t is always 64 bits where this runs, but it avoids warnings in compile test. --- drivers/memory/tegra/mc.c | 25 + drivers/memory/tegra/tegra234.c | 5 + include/soc/tegra/mc.h | 11 +++ 3 files changed, 41 insertions(+) Krzysztof, I've applied this to the same tree as the patch that uses it for now. Let me know if you want me to put this on a separate stable branch for you to pull in. Any update on this? What kind of update do you expect? Ha! I guess I should be more explicit :-) Well, I would like to see this change in -next and so I was hoping that you would respond to the above to indicate how you would like to pull this in. The change will be in next via Thierry. I do not have to pull this in. The maintainer which applies patches is responsible for: 1. Having his tree in linux-next, 2. Sending the patches to upstream maintainer (e.g. arm-soc, Linus) later in pull request. There is no job for me here, if I agree with Thierry. There would be a job if I needed a separate stable branch, but that I did not decide yet... Do you think I need to pull it? If so, why? No. Like I said I just want to get this into -next for testing. I had _wrongly_ assumed that Thierry was waiting on feedback from you. I see this is not the case and so let me check with Thierry where this is. Jon -- nvpublic
Re: [PATCH v3 1/8] memory: tegra: Add API for retrieving carveout bounds
Hi Thierry, Krzysztof, On 24/10/2022 14:15, Thierry Reding wrote: On Tue, Sep 20, 2022 at 11:11:56AM +0300, Mikko Perttunen wrote: From: Mikko Perttunen On Tegra234 NVDEC firmware is loaded from a secure carveout, where it has been loaded by a bootloader. When booting NVDEC, we need to tell it the address of this firmware, which we can determine by checking the starting address of the carveout. As such, add an MC API to query the bounds of carveouts, and add related information on Tegra234. Signed-off-by: Mikko Perttunen --- v2: - Add check for 64-bit phys_addr_t. In practice phys_addr_t is always 64 bits where this runs, but it avoids warnings in compile test. --- drivers/memory/tegra/mc.c | 25 + drivers/memory/tegra/tegra234.c | 5 + include/soc/tegra/mc.h | 11 +++ 3 files changed, 41 insertions(+) Krzysztof, I've applied this to the same tree as the patch that uses it for now. Let me know if you want me to put this on a separate stable branch for you to pull in. Any update on this? Thanks! Jon -- nvpublic
Re: [PATCH v2] gpu: host1x: Avoid trying to use GART on Tegra20
Thierry, On 21/10/2022 08:41, Jon Hunter wrote: On 20/10/2022 15:23, Robin Murphy wrote: Since commit c7e3ca515e78 ("iommu/tegra: gart: Do not register with bus") quite some time ago, the GART driver has effectively disabled itself to avoid issues with the GPU driver expecting it to work in ways that it doesn't. As of commit 57365a04c921 ("iommu: Move bus setup to IOMMU device registration") that bodge no longer works, but really the GPU driver should be responsible for its own behaviour anyway. Make the workaround explicit. Reported-by: Jon Hunter Suggested-by: Dmitry Osipenko Signed-off-by: Robin Murphy --- v2: Cover DRM instance too, move into *_wants_iommu() for consistency drivers/gpu/drm/tegra/drm.c | 4 drivers/gpu/host1x/dev.c | 4 2 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 6748ec1e0005..a1f909dac89a 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -1093,6 +1093,10 @@ static bool host1x_drm_wants_iommu(struct host1x_device *dev) struct host1x *host1x = dev_get_drvdata(dev->dev.parent); struct iommu_domain *domain; + /* Our IOMMU usage policy doesn't currently play well with GART */ + if (of_machine_is_compatible("nvidia,tegra20")) + return false; + /* * If the Tegra DRM clients are backed by an IOMMU, push buffers are * likely to be allocated beyond the 32-bit boundary if sufficient diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index 0cd3f97e7e49..f60ea24db0ec 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -292,6 +292,10 @@ static void host1x_setup_virtualization_tables(struct host1x *host) static bool host1x_wants_iommu(struct host1x *host1x) { + /* Our IOMMU usage policy doesn't currently play well with GART */ + if (of_machine_is_compatible("nvidia,tegra20")) + return false; + /* * If we support addressing a maximum of 32 bits of physical memory * and if the host1x firewall is enabled, there's no need to enable Thanks! Works for me. Tested-by: Jon Hunter We need to pick this fix up for v6.1. Thanks Jon -- nvpublic
Re: [PATCH] gpu: host1x: fix uninitialized variable use
On 08/03/2023 16:56, Nathan Chancellor wrote: Ping? This warning is now in 6.3-rc1. Thierry is away at the moment. David, Daniel, do you want to pick this up directly in the meantime as a fix for 6.3? Mikko has already reviewed and FWIW ... Reviewed-by: Jon Hunter Thanks Jon On Thu, Feb 23, 2023 at 09:28:28AM -0700, Nathan Chancellor wrote: Hi Thierry, Daniel, and David, On Fri, Jan 27, 2023 at 11:14:00PM +0100, Arnd Bergmann wrote: From: Arnd Bergmann The error handling for platform_get_irq() failing no longer works after a recent change, clang now points this out with a warning: drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized when used here [-Werror,-Wuninitialized] if (syncpt_irq < 0) ^~ Fix this by removing the variable and checking the correct error status. Fixes: 625d4ffb438c ("gpu: host1x: Rewrite syncpoint interrupt handling") Signed-off-by: Arnd Bergmann --- drivers/gpu/host1x/dev.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index 4872d183d860..aae2efeef503 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -487,7 +487,6 @@ static int host1x_get_resets(struct host1x *host) static int host1x_probe(struct platform_device *pdev) { struct host1x *host; - int syncpt_irq; int err; host = devm_kzalloc(>dev, sizeof(*host), GFP_KERNEL); @@ -517,8 +516,8 @@ static int host1x_probe(struct platform_device *pdev) } host->syncpt_irq = platform_get_irq(pdev, 0); - if (syncpt_irq < 0) - return syncpt_irq; + if (host->syncpt_irq < 0) + return host->syncpt_irq; mutex_init(>devices_lock); INIT_LIST_HEAD(>devices); -- 2.39.0 Apologies if this has been reported already or has a solution in progress but mainline is now broken because this change got separated from the change it is fixing: https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/4249931209/jobs/7391912774 https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/builds/2M7y9HpiXB13qiC2mkHMyeZOcLW/build.log I see this change sitting in the drm-tegra tree [1], which is getting merged into -next, so it is fixed there, which is why we did not notice any issues until the drm-next tree was merged into mainline. Can this be fast tracked to Linus to unbreak clang builds with -Werror? [1]: https://gitlab.freedesktop.org/drm/tegra/-/commit/b9930311641cf2ed905a84aabe27e8f3868aee4a -- nvpublic
Re: [PATCH] gpu: host1x: Skip reset assert on Tegra186
On 14/02/2024 11:40, Mikko Perttunen wrote: From: Mikko Perttunen On Tegra186, other software components may rely on the kernel to keep Host1x operational even during suspend. As such, as a quirk, skip asserting Host1x's reset on Tegra186. We don't need to keep the clocks enabled, as BPMP ensures the clock stays on while Host1x is being used. On newer SoC's, the reset line is inaccessible, so there is no need for the quirk. Signed-off-by: Mikko Perttunen We should add the fixes tag ... Fixes: b7c00cdf6df5 ("gpu: host1x: Enable system suspend callbacks") ... because this fixes a suspend regression on Tegra186. Thierry, would you be able to add the fixes-tag and send out as a fix for v6.8? Otherwise ... Reviewed-by: Jon Hunter Tested-by: Jon Hunter Thanks! Jon -- nvpublic
Re: [PATCH] drm/nouveau: Fixup gk20a instobj hierarchy
On 08/12/2023 10:46, Thierry Reding wrote: From: Thierry Reding Commit 12c9b05da918 ("drm/nouveau/imem: support allocations not preserved across suspend") uses container_of() to cast from struct nvkm_memory to struct nvkm_instobj, assuming that all instance objects are derived from struct nvkm_instobj. For the gk20a family that's not the case and they are derived from struct nvkm_memory instead. This causes some subtle data corruption (nvkm_instobj.preserve ends up mapping to gk20a_instobj.vaddr) that causes a NULL pointer dereference in gk20a_instobj_acquire_iommu() (and possibly elsewhere) and also prevents suspend/resume from working. Fix this by making struct gk20a_instobj derive from struct nvkm_instobj instead. Fixes: 12c9b05da918 ("drm/nouveau/imem: support allocations not preserved across suspend") Reported-by: Jonathan Hunter Signed-off-by: Thierry Reding --- Note that this was probably subtly wrong before the above-mentioned commit already, but I don't think we've seen any reports that would indicate any actual failures related to this before. So I think it's good enough to apply this fix for v6.7. The next closest thing would be commit d8e83994aaf6 ("drm/nouveau/imem: improve management of instance memory"), but that's 8 years old (Linux v4.3)... --- .../drm/nouveau/nvkm/subdev/instmem/gk20a.c| 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c index 1b811d6972a1..201022ae9214 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c @@ -49,14 +49,14 @@ #include struct gk20a_instobj { - struct nvkm_memory memory; + struct nvkm_instobj base; struct nvkm_mm_node *mn; struct gk20a_instmem *imem; /* CPU mapping */ u32 *vaddr; }; -#define gk20a_instobj(p) container_of((p), struct gk20a_instobj, memory) +#define gk20a_instobj(p) container_of((p), struct gk20a_instobj, base.memory) /* * Used for objects allocated using the DMA API @@ -148,7 +148,7 @@ gk20a_instobj_iommu_recycle_vaddr(struct gk20a_instobj_iommu *obj) list_del(>vaddr_node); vunmap(obj->base.vaddr); obj->base.vaddr = NULL; - imem->vaddr_use -= nvkm_memory_size(>base.memory); + imem->vaddr_use -= nvkm_memory_size(>base.base.memory); nvkm_debug(>base.subdev, "vaddr used: %x/%x\n", imem->vaddr_use, imem->vaddr_max); } @@ -283,7 +283,7 @@ gk20a_instobj_map(struct nvkm_memory *memory, u64 offset, struct nvkm_vmm *vmm, { struct gk20a_instobj *node = gk20a_instobj(memory); struct nvkm_vmm_map map = { - .memory = >memory, + .memory = >base.memory, .offset = offset, .mem = node->mn, }; @@ -391,8 +391,8 @@ gk20a_instobj_ctor_dma(struct gk20a_instmem *imem, u32 npages, u32 align, return -ENOMEM; *_node = >base; - nvkm_memory_ctor(_instobj_func_dma, >base.memory); - node->base.memory.ptrs = _instobj_ptrs; + nvkm_memory_ctor(_instobj_func_dma, >base.base.memory); + node->base.base.memory.ptrs = _instobj_ptrs; node->base.vaddr = dma_alloc_attrs(dev, npages << PAGE_SHIFT, >handle, GFP_KERNEL, @@ -438,8 +438,8 @@ gk20a_instobj_ctor_iommu(struct gk20a_instmem *imem, u32 npages, u32 align, *_node = >base; node->dma_addrs = (void *)(node->pages + npages); - nvkm_memory_ctor(_instobj_func_iommu, >base.memory); - node->base.memory.ptrs = _instobj_ptrs; + nvkm_memory_ctor(_instobj_func_iommu, >base.base.memory); + node->base.base.memory.ptrs = _instobj_ptrs; /* Allocate backing memory */ for (i = 0; i < npages; i++) { @@ -533,7 +533,7 @@ gk20a_instobj_new(struct nvkm_instmem *base, u32 size, u32 align, bool zero, else ret = gk20a_instobj_ctor_dma(imem, size >> PAGE_SHIFT, align, ); - *pmemory = node ? >memory : NULL; + *pmemory = node ? >base.memory : NULL; if (ret) return ret; Tested-by: Jon Hunter Thanks! Jon -- nvpublic
Re: [PATCH] drm/tegra: Remove of_dma_configure() from host1x_device_add()
On 31/01/2024 15:33, Jason Gunthorpe wrote: On Tue, Jan 30, 2024 at 09:55:18PM +, Jon Hunter wrote: On 30/01/2024 16:15, Jason Gunthorpe wrote: This was added in commit c95469aa5a18 ("gpu: host1x: Set DMA ops on device creation") with the note: Currently host1x-instanciated devices have their dma_ops left to NULL, which makes any DMA operation (like buffer import) on ARM64 fallback to the dummy_dma_ops and fail with an error. Since commit 14891af3799e ("iommu: Move the iommu driver sysfs setup into iommu_init/deinit_device()") this call now fails because the struct device is not fully configured enough to setup the sysfs and we now catch that error. This failure means the DMA ops are no longer set during this failing call. Looking at it more it seems the arch dma ops are setup still, we ignore the failure on multiple levels :( It seems this is no longer a problem because commit 07397df29e57 ("dma-mapping: move dma configuration to bus infrastructure") added another call to of_dma_configure() inside the bus_type->dma_configure() callback. So long as a driver is probed the to the device it will have DMA properly setup in the ordinary way. Remove the unnecessary call which also removes the new long print: [1.24] host1x drm: iommu configuration for device failed with -ENOENT Reported-by: Diogo Ivo Closes: https://lore.kernel.org/all/bbmhcoghrprmbdibnjum6lefix2eoquxrde7wyqeulm4xabmlm@b6jy32saugqh/ Reported-by: Jon Hunter Closes: https://lore.kernel.org/all/b0334c5e-3a6c-4b58-b525-e72bed889...@nvidia.com/ Signed-off-by: Jason Gunthorpe --- drivers/gpu/host1x/bus.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c index 84d042796d2e66..61214d35cadc34 100644 --- a/drivers/gpu/host1x/bus.c +++ b/drivers/gpu/host1x/bus.c @@ -458,8 +458,6 @@ static int host1x_device_add(struct host1x *host1x, device->dev.bus = _bus_type; device->dev.parent = host1x->dev; - of_dma_configure(>dev, host1x->dev->of_node, true); - device->dev.dma_parms = >dma_parms; dma_set_max_seg_size(>dev, UINT_MAX); In my case the warning is coming from the of_dma_configure_id() in drivers/gpu/host1x/context.c. So with the above change I am still seeing the warning. You mean this sequence? err = device_add(>dev); if (err) { dev_err(host1x->dev, "could not add context device %d: %d\n", i, err); put_device(>dev); goto unreg_devices; } err = of_dma_configure_id(>dev, node, true, ); if (err) { dev_err(host1x->dev, "IOMMU configuration failed for context device %d: %d\n", i, err); device_unregister(>dev); goto unreg_devices; } Yes this sequence. I didn't seem an obvious place that this would get fixed up later? device_add() was done before so the iommu_device_link() shouldn't be failing? Are you hitting a duplicate link (ie remove the nowarn from iommu_device_link()) Removing the '_nowarn' does appear to fix it, although it is not clear to me why? Thanks Jon -- nvpublic
Re: [PATCH] drm/tegra: Remove of_dma_configure() from host1x_device_add()
On 01/02/2024 20:02, Jason Gunthorpe wrote: On Thu, Feb 01, 2024 at 07:35:24PM +, Jon Hunter wrote: You mean this sequence? err = device_add(>dev); if (err) { dev_err(host1x->dev, "could not add context device %d: %d\n", i, err); put_device(>dev); goto unreg_devices; } err = of_dma_configure_id(>dev, node, true, ); if (err) { dev_err(host1x->dev, "IOMMU configuration failed for context device %d: %d\n", i, err); device_unregister(>dev); goto unreg_devices; } Yes this sequence. I didn't seem an obvious place that this would get fixed up later? device_add() was done before so the iommu_device_link() shouldn't be failing? Are you hitting a duplicate link (ie remove the nowarn from iommu_device_link()) Removing the '_nowarn' does appear to fix it, although it is not clear to me why? What did you do to remove? Just the letters or the whole line? Yes sorry, to be clear I made the following change ... diff --git a/drivers/iommu/iommu-sysfs.c b/drivers/iommu/iommu-sysfs.c index cbe378c34ba3..7bc9c6d2baab 100644 --- a/drivers/iommu/iommu-sysfs.c +++ b/drivers/iommu/iommu-sysfs.c @@ -112,7 +112,7 @@ int iommu_device_link(struct iommu_device *iommu, struct device *link) if (ret) return ret; - ret = sysfs_create_link_nowarn(>kobj, >dev->kobj, "iommu"); + ret = sysfs_create_link(>kobj, >dev->kobj, "iommu"); if (ret) I was thinking the letters because it triggers a large debug message. But, what is the actual log output you see, is it -EEXIST? I see ... ERR KERN host1x drm: iommu configuration for device failed with -ENOENT If it is coming and going is it a race of some kind? It is consistent without the above. However, I did not think that the above change would change the returning on -ENOENT? I will add more debug. Jon -- nvpublic
Re: [PATCH] drm/tegra: Remove of_dma_configure() from host1x_device_add()
On 02/02/2024 14:35, Jason Gunthorpe wrote: On Fri, Feb 02, 2024 at 10:40:36AM +, Jon Hunter wrote: But, what is the actual log output you see, is it -EEXIST? I see ... ERR KERN host1x drm: iommu configuration for device failed with -ENOENT So that shouldn't happen in you case as far as I can tell, the device is properly added, link->kobj should be fine and ENOENT shouldn't happen. If it is coming and going is it a race of some kind? It is consistent without the above. However, I did not think that the above change would change the returning on -ENOENT? I will add more debug. I do not think it can either Still wonder if there is some odd race.. Let me know when you figure out what is happening - I think there is some bug here it is not just a harmless warning. Yes looks like a race of some sort. Adding a bit of debug also makes the issue go away so difficult to see what is happening. Jon -- nvpublic
Re: [PATCH] drm/tegra: Remove of_dma_configure() from host1x_device_add()
On 30/01/2024 16:15, Jason Gunthorpe wrote: This was added in commit c95469aa5a18 ("gpu: host1x: Set DMA ops on device creation") with the note: Currently host1x-instanciated devices have their dma_ops left to NULL, which makes any DMA operation (like buffer import) on ARM64 fallback to the dummy_dma_ops and fail with an error. Since commit 14891af3799e ("iommu: Move the iommu driver sysfs setup into iommu_init/deinit_device()") this call now fails because the struct device is not fully configured enough to setup the sysfs and we now catch that error. This failure means the DMA ops are no longer set during this failing call. It seems this is no longer a problem because commit 07397df29e57 ("dma-mapping: move dma configuration to bus infrastructure") added another call to of_dma_configure() inside the bus_type->dma_configure() callback. So long as a driver is probed the to the device it will have DMA properly setup in the ordinary way. Remove the unnecessary call which also removes the new long print: [1.24] host1x drm: iommu configuration for device failed with -ENOENT Reported-by: Diogo Ivo Closes: https://lore.kernel.org/all/bbmhcoghrprmbdibnjum6lefix2eoquxrde7wyqeulm4xabmlm@b6jy32saugqh/ Reported-by: Jon Hunter Closes: https://lore.kernel.org/all/b0334c5e-3a6c-4b58-b525-e72bed889...@nvidia.com/ Signed-off-by: Jason Gunthorpe --- drivers/gpu/host1x/bus.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c index 84d042796d2e66..61214d35cadc34 100644 --- a/drivers/gpu/host1x/bus.c +++ b/drivers/gpu/host1x/bus.c @@ -458,8 +458,6 @@ static int host1x_device_add(struct host1x *host1x, device->dev.bus = _bus_type; device->dev.parent = host1x->dev; - of_dma_configure(>dev, host1x->dev->of_node, true); - device->dev.dma_parms = >dma_parms; dma_set_max_seg_size(>dev, UINT_MAX); In my case the warning is coming from the of_dma_configure_id() in drivers/gpu/host1x/context.c. So with the above change I am still seeing the warning. BTW, the subject prefix should be 'gpu: host1x:' for this change. Jon -- nvpublic
Re: [PATCH v5 11/18] drm/msm: generate headers on the fly
Hi Dmitry, On 01/04/2024 03:42, Dmitry Baryshkov wrote: Generate DRM/MSM headers on the fly during kernel build. This removes a need to push register changes to Mesa with the following manual synchronization step. Existing headers will be removed in the following commits (split away to ease reviews). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/.gitignore | 1 + drivers/gpu/drm/msm/Makefile | 97 +- 2 files changed, 77 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/msm/.gitignore b/drivers/gpu/drm/msm/.gitignore new file mode 100644 index ..9ab870da897d --- /dev/null +++ b/drivers/gpu/drm/msm/.gitignore @@ -0,0 +1 @@ +generated/ diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 26ed4f443149..c861de58286c 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -1,10 +1,11 @@ # SPDX-License-Identifier: GPL-2.0 ccflags-y := -I $(srctree)/$(src) +ccflags-y += -I $(obj)/generated ccflags-y += -I $(srctree)/$(src)/disp/dpu1 ccflags-$(CONFIG_DRM_MSM_DSI) += -I $(srctree)/$(src)/dsi ccflags-$(CONFIG_DRM_MSM_DP) += -I $(srctree)/$(src)/dp -msm-y := \ +adreno-y := \ adreno/adreno_device.o \ adreno/adreno_gpu.o \ adreno/a2xx_gpu.o \ @@ -18,7 +19,11 @@ msm-y := \ adreno/a6xx_gmu.o \ adreno/a6xx_hfi.o \ -msm-$(CONFIG_DRM_MSM_HDMI) += \ +adreno-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \ + +adreno-$(CONFIG_DRM_MSM_GPU_STATE) += adreno/a6xx_gpu_state.o + +msm-display-$(CONFIG_DRM_MSM_HDMI) += \ hdmi/hdmi.o \ hdmi/hdmi_audio.o \ hdmi/hdmi_bridge.o \ @@ -31,7 +36,7 @@ msm-$(CONFIG_DRM_MSM_HDMI) += \ hdmi/hdmi_phy_8x74.o \ hdmi/hdmi_pll_8960.o \ -msm-$(CONFIG_DRM_MSM_MDP4) += \ +msm-display-$(CONFIG_DRM_MSM_MDP4) += \ disp/mdp4/mdp4_crtc.o \ disp/mdp4/mdp4_dsi_encoder.o \ disp/mdp4/mdp4_dtv_encoder.o \ @@ -42,7 +47,7 @@ msm-$(CONFIG_DRM_MSM_MDP4) += \ disp/mdp4/mdp4_kms.o \ disp/mdp4/mdp4_plane.o \ -msm-$(CONFIG_DRM_MSM_MDP5) += \ +msm-display-$(CONFIG_DRM_MSM_MDP5) += \ disp/mdp5/mdp5_cfg.o \ disp/mdp5/mdp5_cmd_encoder.o \ disp/mdp5/mdp5_ctl.o \ @@ -55,7 +60,7 @@ msm-$(CONFIG_DRM_MSM_MDP5) += \ disp/mdp5/mdp5_plane.o \ disp/mdp5/mdp5_smp.o \ -msm-$(CONFIG_DRM_MSM_DPU) += \ +msm-display-$(CONFIG_DRM_MSM_DPU) += \ disp/dpu1/dpu_core_perf.o \ disp/dpu1/dpu_crtc.o \ disp/dpu1/dpu_encoder.o \ @@ -85,14 +90,16 @@ msm-$(CONFIG_DRM_MSM_DPU) += \ disp/dpu1/dpu_vbif.o \ disp/dpu1/dpu_writeback.o -msm-$(CONFIG_DRM_MSM_MDSS) += \ +msm-display-$(CONFIG_DRM_MSM_MDSS) += \ msm_mdss.o \ -msm-y += \ +msm-display-y += \ disp/mdp_format.o \ disp/mdp_kms.o \ disp/msm_disp_snapshot.o \ disp/msm_disp_snapshot_util.o \ + +msm-y += \ msm_atomic.o \ msm_atomic_tracepoints.o \ msm_debugfs.o \ @@ -115,12 +122,12 @@ msm-y += \ msm_submitqueue.o \ msm_gpu_tracepoints.o \ -msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \ - dp/dp_debug.o +msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o -msm-$(CONFIG_DRM_MSM_GPU_STATE) += adreno/a6xx_gpu_state.o +msm-display-$(CONFIG_DEBUG_FS) += \ + dp/dp_debug.o -msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \ +msm-display-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \ dp/dp_catalog.o \ dp/dp_ctrl.o \ dp/dp_display.o \ @@ -130,21 +137,69 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \ dp/dp_audio.o \ dp/dp_utils.o -msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o - -msm-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o +msm-display-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o -msm-$(CONFIG_DRM_MSM_DSI) += dsi/dsi.o \ +msm-display-$(CONFIG_DRM_MSM_DSI) += dsi/dsi.o \ dsi/dsi_cfg.o \ dsi/dsi_host.o \ dsi/dsi_manager.o \ dsi/phy/dsi_phy.o -msm-$(CONFIG_DRM_MSM_DSI_28NM_PHY) += dsi/phy/dsi_phy_28nm.o -msm-$(CONFIG_DRM_MSM_DSI_20NM_PHY) += dsi/phy/dsi_phy_20nm.o -msm-$(CONFIG_DRM_MSM_DSI_28NM_8960_PHY) += dsi/phy/dsi_phy_28nm_8960.o -msm-$(CONFIG_DRM_MSM_DSI_14NM_PHY) += dsi/phy/dsi_phy_14nm.o -msm-$(CONFIG_DRM_MSM_DSI_10NM_PHY) += dsi/phy/dsi_phy_10nm.o -msm-$(CONFIG_DRM_MSM_DSI_7NM_PHY) += dsi/phy/dsi_phy_7nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_28NM_PHY) += dsi/phy/dsi_phy_28nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_20NM_PHY) += dsi/phy/dsi_phy_20nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_28NM_8960_PHY) += dsi/phy/dsi_phy_28nm_8960.o +msm-display-$(CONFIG_DRM_MSM_DSI_14NM_PHY) += dsi/phy/dsi_phy_14nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_10NM_PHY) += dsi/phy/dsi_phy_10nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_7NM_PHY) += dsi/phy/dsi_phy_7nm.o + +msm-y += $(adreno-y) $(msm-display-y) obj-$(CONFIG_DRM_MSM) += msm.o +
[PATCH] drm/msm: Fix gen_header.py for older python3 versions
The gen_header.py script is failing for older versions of python3 such as python 3.5. Two issues observed with python 3.5 are ... 1. Python 3 versions prior to 3.6 do not support the f-string format. 2. Early python 3 versions do not support the 'required' argument for the argparse add_subparsers(). Fix both of the above so that older versions of python 3 still work. Fixes: 8f7abf0b86fe ("drm/msm: generate headers on the fly") Signed-off-by: Jon Hunter --- drivers/gpu/drm/msm/registers/gen_header.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/registers/gen_header.py b/drivers/gpu/drm/msm/registers/gen_header.py index 9b2842d4a354..90d5c2991d05 100644 --- a/drivers/gpu/drm/msm/registers/gen_header.py +++ b/drivers/gpu/drm/msm/registers/gen_header.py @@ -323,7 +323,7 @@ class Array(object): indices = [] if self.length != 1: if self.fixed_offsets: - indices.append((self.index_ctype(), None, f"__offset_{self.local_name}")) + indices.append((self.index_ctype(), None, "__offset_%s" % self.local_name)) else: indices.append((self.index_ctype(), self.stride, None)) return indices @@ -942,7 +942,8 @@ def main(): parser.add_argument('--rnn', type=str, required=True) parser.add_argument('--xml', type=str, required=True) - subparsers = parser.add_subparsers(required=True) + subparsers = parser.add_subparsers() + subparsers.required = True parser_c_defines = subparsers.add_parser('c-defines') parser_c_defines.set_defaults(func=dump_c_defines) -- 2.34.1
Re: [PATCH v5 11/18] drm/msm: generate headers on the fly
On 12/04/2024 17:19, Dmitry Baryshkov wrote: On Fri, 12 Apr 2024 at 19:15, Jon Hunter wrote: Hi Dmitry, On 01/04/2024 03:42, Dmitry Baryshkov wrote: Generate DRM/MSM headers on the fly during kernel build. This removes a need to push register changes to Mesa with the following manual synchronization step. Existing headers will be removed in the following commits (split away to ease reviews). Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/.gitignore | 1 + drivers/gpu/drm/msm/Makefile | 97 +- 2 files changed, 77 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/msm/.gitignore b/drivers/gpu/drm/msm/.gitignore new file mode 100644 index ..9ab870da897d --- /dev/null +++ b/drivers/gpu/drm/msm/.gitignore @@ -0,0 +1 @@ +generated/ diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 26ed4f443149..c861de58286c 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -1,10 +1,11 @@ # SPDX-License-Identifier: GPL-2.0 ccflags-y := -I $(srctree)/$(src) +ccflags-y += -I $(obj)/generated ccflags-y += -I $(srctree)/$(src)/disp/dpu1 ccflags-$(CONFIG_DRM_MSM_DSI) += -I $(srctree)/$(src)/dsi ccflags-$(CONFIG_DRM_MSM_DP) += -I $(srctree)/$(src)/dp -msm-y := \ +adreno-y := \ adreno/adreno_device.o \ adreno/adreno_gpu.o \ adreno/a2xx_gpu.o \ @@ -18,7 +19,11 @@ msm-y := \ adreno/a6xx_gmu.o \ adreno/a6xx_hfi.o \ -msm-$(CONFIG_DRM_MSM_HDMI) += \ +adreno-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \ + +adreno-$(CONFIG_DRM_MSM_GPU_STATE) += adreno/a6xx_gpu_state.o + +msm-display-$(CONFIG_DRM_MSM_HDMI) += \ hdmi/hdmi.o \ hdmi/hdmi_audio.o \ hdmi/hdmi_bridge.o \ @@ -31,7 +36,7 @@ msm-$(CONFIG_DRM_MSM_HDMI) += \ hdmi/hdmi_phy_8x74.o \ hdmi/hdmi_pll_8960.o \ -msm-$(CONFIG_DRM_MSM_MDP4) += \ +msm-display-$(CONFIG_DRM_MSM_MDP4) += \ disp/mdp4/mdp4_crtc.o \ disp/mdp4/mdp4_dsi_encoder.o \ disp/mdp4/mdp4_dtv_encoder.o \ @@ -42,7 +47,7 @@ msm-$(CONFIG_DRM_MSM_MDP4) += \ disp/mdp4/mdp4_kms.o \ disp/mdp4/mdp4_plane.o \ -msm-$(CONFIG_DRM_MSM_MDP5) += \ +msm-display-$(CONFIG_DRM_MSM_MDP5) += \ disp/mdp5/mdp5_cfg.o \ disp/mdp5/mdp5_cmd_encoder.o \ disp/mdp5/mdp5_ctl.o \ @@ -55,7 +60,7 @@ msm-$(CONFIG_DRM_MSM_MDP5) += \ disp/mdp5/mdp5_plane.o \ disp/mdp5/mdp5_smp.o \ -msm-$(CONFIG_DRM_MSM_DPU) += \ +msm-display-$(CONFIG_DRM_MSM_DPU) += \ disp/dpu1/dpu_core_perf.o \ disp/dpu1/dpu_crtc.o \ disp/dpu1/dpu_encoder.o \ @@ -85,14 +90,16 @@ msm-$(CONFIG_DRM_MSM_DPU) += \ disp/dpu1/dpu_vbif.o \ disp/dpu1/dpu_writeback.o -msm-$(CONFIG_DRM_MSM_MDSS) += \ +msm-display-$(CONFIG_DRM_MSM_MDSS) += \ msm_mdss.o \ -msm-y += \ +msm-display-y += \ disp/mdp_format.o \ disp/mdp_kms.o \ disp/msm_disp_snapshot.o \ disp/msm_disp_snapshot_util.o \ + +msm-y += \ msm_atomic.o \ msm_atomic_tracepoints.o \ msm_debugfs.o \ @@ -115,12 +122,12 @@ msm-y += \ msm_submitqueue.o \ msm_gpu_tracepoints.o \ -msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \ - dp/dp_debug.o +msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o -msm-$(CONFIG_DRM_MSM_GPU_STATE) += adreno/a6xx_gpu_state.o +msm-display-$(CONFIG_DEBUG_FS) += \ + dp/dp_debug.o -msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \ +msm-display-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \ dp/dp_catalog.o \ dp/dp_ctrl.o \ dp/dp_display.o \ @@ -130,21 +137,69 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \ dp/dp_audio.o \ dp/dp_utils.o -msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o - -msm-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o +msm-display-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o -msm-$(CONFIG_DRM_MSM_DSI) += dsi/dsi.o \ +msm-display-$(CONFIG_DRM_MSM_DSI) += dsi/dsi.o \ dsi/dsi_cfg.o \ dsi/dsi_host.o \ dsi/dsi_manager.o \ dsi/phy/dsi_phy.o -msm-$(CONFIG_DRM_MSM_DSI_28NM_PHY) += dsi/phy/dsi_phy_28nm.o -msm-$(CONFIG_DRM_MSM_DSI_20NM_PHY) += dsi/phy/dsi_phy_20nm.o -msm-$(CONFIG_DRM_MSM_DSI_28NM_8960_PHY) += dsi/phy/dsi_phy_28nm_8960.o -msm-$(CONFIG_DRM_MSM_DSI_14NM_PHY) += dsi/phy/dsi_phy_14nm.o -msm-$(CONFIG_DRM_MSM_DSI_10NM_PHY) += dsi/phy/dsi_phy_10nm.o -msm-$(CONFIG_DRM_MSM_DSI_7NM_PHY) += dsi/phy/dsi_phy_7nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_28NM_PHY) += dsi/phy/dsi_phy_28nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_20NM_PHY) += dsi/phy/dsi_phy_20nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_28NM_8960_PHY) += dsi/phy/dsi_phy_28nm_8960.o +msm-display-$(CONFIG_DRM_MSM_DSI_14NM_PHY) += dsi/phy/dsi_phy_14nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_10NM_PHY) += dsi/phy/dsi_phy_10nm.o +msm-display-$(CONFIG_DRM_MSM_DSI_7NM_PHY) += dsi/phy/dsi_phy_7nm.o + +msm-y += $(adreno-y) $(msm-display-y) obj
Re: [PATCH] drm/msm: Fix gen_header.py for older python3 versions
Hi all, On 12/04/2024 17:54, Jon Hunter wrote: The gen_header.py script is failing for older versions of python3 such as python 3.5. Two issues observed with python 3.5 are ... 1. Python 3 versions prior to 3.6 do not support the f-string format. 2. Early python 3 versions do not support the 'required' argument for the argparse add_subparsers(). Fix both of the above so that older versions of python 3 still work. Fixes: 8f7abf0b86fe ("drm/msm: generate headers on the fly") Signed-off-by: Jon Hunter --- drivers/gpu/drm/msm/registers/gen_header.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/registers/gen_header.py b/drivers/gpu/drm/msm/registers/gen_header.py index 9b2842d4a354..90d5c2991d05 100644 --- a/drivers/gpu/drm/msm/registers/gen_header.py +++ b/drivers/gpu/drm/msm/registers/gen_header.py @@ -323,7 +323,7 @@ class Array(object): indices = [] if self.length != 1: if self.fixed_offsets: - indices.append((self.index_ctype(), None, f"__offset_{self.local_name}")) + indices.append((self.index_ctype(), None, "__offset_%s" % self.local_name)) else: indices.append((self.index_ctype(), self.stride, None)) return indices @@ -942,7 +942,8 @@ def main(): parser.add_argument('--rnn', type=str, required=True) parser.add_argument('--xml', type=str, required=True) - subparsers = parser.add_subparsers(required=True) + subparsers = parser.add_subparsers() + subparsers.required = True parser_c_defines = subparsers.add_parser('c-defines') parser_c_defines.set_defaults(func=dump_c_defines) Any feedback on this? All our farm builders are still broken :-( Thanks Jon -- nvpublic
Re: [PATCH] drm/msm: Fix gen_header.py for python earlier than v3.9
On 08/05/2024 17:46, Abhinav Kumar wrote: On 5/8/2024 2:17 AM, Jon Hunter wrote: Building the kernel with python3 versions earlier than v3.9 fails with ... Traceback (most recent call last): File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in main() File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main parser.add_argument('--validate', action=argparse.BooleanOptionalAction) AttributeError: module 'argparse' has no attribute 'BooleanOptionalAction' The argparse attribute 'BooleanOptionalAction' is only supported for python v3.9 and later. Fix support for earlier python3 versions by explicitly defining '--validate' and '--no-validate' arguments. Signed-off-by: Jon Hunter --- drivers/gpu/drm/msm/registers/gen_header.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Thanks for your patch, I had sent something similar y'day. If you are alright with https://patchwork.freedesktop.org/patch/593057/, we can use that one. Yes that's fine with me. Thanks Jon -- nvpublic
Re: [PATCH] docs: document python version used for compilation
On 10/05/2024 12:39, Mauro Carvalho Chehab wrote: Em Fri, 10 May 2024 13:39:17 +0300 Dmitry Baryshkov escreveu: On Fri, 10 May 2024 at 13:09, Jani Nikula wrote: On Fri, 10 May 2024, Mauro Carvalho Chehab wrote: Em Fri, 10 May 2024 11:08:38 +0300 Jani Nikula escreveu: On Thu, 09 May 2024, Dmitry Baryshkov wrote: The drm/msm driver had adopted using Python3 script to generate register header files instead of shipping pre-generated header files. Document the minimal Python version supported by the script. Signed-off-by: Dmitry Baryshkov --- Documentation/process/changes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/process/changes.rst b/Documentation/process/changes.rst index 5685d7bfe4d0..8d225a9f65a2 100644 --- a/Documentation/process/changes.rst +++ b/Documentation/process/changes.rst @@ -63,6 +63,7 @@ cpio any cpio --version GNU tar1.28 tar --version gtags (optional) 6.6.5gtags --version mkimage (optional) 2017.01 mkimage --version +Python (optional) 3.5.xpython3 --version Python 3.5 reached end-of-life 3½ years ago [1]. What's the point in using anything older than the oldest supported version of Python, i.e. 3.8 at this time? What's the point of breaking compilation with on older distros? The idea of minimal versions here is to specify the absolute minimum version that it is required for the build to happen. If 3.5 is the minimal one, then be it. AFAICT 3.5 was an arbitrary rather than a deliberate choice. We should at least be aware *why* we'd be sticking to old versions. From my side, the 3.5 was chosen basing on the previous feedback from Jon Hunter: https://lore.kernel.org/dri-devel/20240412165407.42163-1-jonath...@nvidia.com/ Patch there seems small/simple enough if it is all it takes for 3.5. Yet, it would be nice to hear from Jon Hunter about the rationale for 3.5 support (if any). We just have some legacy builders for legacy Tegra devices that are still using python 3.5. I will request that these are updated but these are not machines that I own and so may take some time. Minimum versions here also means sticking to features available in said versions, for Python just as well as for GCC or any other tool. That's not zero cost. I guess there are two angles here too. The absolute minimum version currently required, and the, uh, maximum the minimum version can be safely bumped to. Say, you want to use a feature not available in the current minimum, how far up can you bump the version to? Could we define and document the criteria (e.g. based on distros as you suggest below) so we don't have to repeat the discussion? Agreed. While we should not bump version randomly, defining a criteria about when we should update the requirement sounds a great idea. For me, the criteria is: - the minimal version shall be at least the minimal one required for the Kernel to build at the most used LTS distros that are not EOL, e. g.: Debian, openSUSE/SUSE, CentOS/RHEL and Ubuntu LTS[1]. [1] In practice, Ubuntu LTS usually has a python version newer than Debian LTS, and CentOS versions are identical to RHEL ones, so I guess checking for Debian, openSUSE, SUSE and RHEL should be enough. Adding Stefan from SUSE because Stefan also reported a similar issue [0]. Note that subject of this email is incorrect and should be python 3.6 and not 2.6 :-) Jon [0] https://lore.kernel.org/all/20240118123752.bl3qss3qbbxgv...@suse.de/ -- nvpublic
[PATCH] drm/msm: Fix gen_header.py for python earlier than v3.9
Building the kernel with python3 versions earlier than v3.9 fails with ... Traceback (most recent call last): File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in main() File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main parser.add_argument('--validate', action=argparse.BooleanOptionalAction) AttributeError: module 'argparse' has no attribute 'BooleanOptionalAction' The argparse attribute 'BooleanOptionalAction' is only supported for python v3.9 and later. Fix support for earlier python3 versions by explicitly defining '--validate' and '--no-validate' arguments. Signed-off-by: Jon Hunter --- drivers/gpu/drm/msm/registers/gen_header.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/registers/gen_header.py b/drivers/gpu/drm/msm/registers/gen_header.py index fc3bfdc991d2..64f67d2e3f1c 100644 --- a/drivers/gpu/drm/msm/registers/gen_header.py +++ b/drivers/gpu/drm/msm/registers/gen_header.py @@ -948,7 +948,8 @@ def main(): parser = argparse.ArgumentParser() parser.add_argument('--rnn', type=str, required=True) parser.add_argument('--xml', type=str, required=True) - parser.add_argument('--validate', action=argparse.BooleanOptionalAction) + parser.add_argument('--validate', dest='validate', action='store_true') + parser.add_argument('--no-validate', dest='validate', action='store_false') subparsers = parser.add_subparsers() subparsers.required = True -- 2.34.1
Re: [PATCH v2] docs: document python version used for compilation
On 12/05/2024 00:52, Abhinav Kumar wrote: On 5/11/2024 3:32 PM, Dmitry Baryshkov wrote: The drm/msm driver had adopted using Python3 script to generate register header files instead of shipping pre-generated header files. Document the minimal Python version supported by the script. Per request by Jon Hunter, the script is required to be compatible with Python 3.5. Python is documented as an optional dependency, as it is required only in a limited set of kernel configurations (following the example of other optional dependencies). Cc: Jon Hunter Signed-off-by: Dmitry Baryshkov --- Depends: https://lore.kernel.org/dri-devel/20240507230440.3384949-1-quic_abhin...@quicinc.com/ --- Changes in v2: - Expanded documentation for the Python usage. - Link to v1: https://lore.kernel.org/r/20240509-python-version-v1-1-a7dda3a95...@linaro.org --- Documentation/process/changes.rst | 8 1 file changed, 8 insertions(+) Reviewed-by: Abhinav Kumar Thanks! I made a request to update some of our legacy builders and so hopefully they might be upgraded to a newer version. Anyway ... Reviewed-by: Jon Hunter Cheers Jon -- nvpublic
Re: [PATCH] gpu: host1x: Do not setup DMA for virtual devices
On 14/03/2024 15:49, Thierry Reding wrote: From: Thierry Reding The host1x devices are virtual compound devices and do not perform DMA accesses themselves, so they do not need to be set up for DMA. Ideally we would also not need to set up DMA masks for the virtual devices, but we currently still need those for legacy support on old hardware. Signed-off-by: Thierry Reding --- drivers/gpu/host1x/bus.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c index 783975d1384f..7c52757a89db 100644 --- a/drivers/gpu/host1x/bus.c +++ b/drivers/gpu/host1x/bus.c @@ -351,11 +351,6 @@ static int host1x_device_uevent(const struct device *dev, return 0; } -static int host1x_dma_configure(struct device *dev) -{ - return of_dma_configure(dev, dev->of_node, true); -} - static const struct dev_pm_ops host1x_device_pm_ops = { .suspend = pm_generic_suspend, .resume = pm_generic_resume, @@ -369,7 +364,6 @@ const struct bus_type host1x_bus_type = { .name = "host1x", .match = host1x_device_match, .uevent = host1x_device_uevent, - .dma_configure = host1x_dma_configure, .pm = _device_pm_ops, }; @@ -458,8 +452,6 @@ static int host1x_device_add(struct host1x *host1x, device->dev.bus = _bus_type; device->dev.parent = host1x->dev; - of_dma_configure(>dev, host1x->dev->of_node, true); - device->dev.dma_parms = >dma_parms; dma_set_max_seg_size(>dev, UINT_MAX); Tested-by: Jon Hunter Acked-by: Jon Hunter Thanks! Jon -- nvpublic
Re: [PATCH] gpu: host1x: Do not setup DMA for virtual devices
Hi Thierry, On 15/03/2024 11:25, Jon Hunter wrote: On 14/03/2024 15:49, Thierry Reding wrote: From: Thierry Reding The host1x devices are virtual compound devices and do not perform DMA accesses themselves, so they do not need to be set up for DMA. Ideally we would also not need to set up DMA masks for the virtual devices, but we currently still need those for legacy support on old hardware. Signed-off-by: Thierry Reding --- drivers/gpu/host1x/bus.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c index 783975d1384f..7c52757a89db 100644 --- a/drivers/gpu/host1x/bus.c +++ b/drivers/gpu/host1x/bus.c @@ -351,11 +351,6 @@ static int host1x_device_uevent(const struct device *dev, return 0; } -static int host1x_dma_configure(struct device *dev) -{ - return of_dma_configure(dev, dev->of_node, true); -} - static const struct dev_pm_ops host1x_device_pm_ops = { .suspend = pm_generic_suspend, .resume = pm_generic_resume, @@ -369,7 +364,6 @@ const struct bus_type host1x_bus_type = { .name = "host1x", .match = host1x_device_match, .uevent = host1x_device_uevent, - .dma_configure = host1x_dma_configure, .pm = _device_pm_ops, }; @@ -458,8 +452,6 @@ static int host1x_device_add(struct host1x *host1x, device->dev.bus = _bus_type; device->dev.parent = host1x->dev; - of_dma_configure(>dev, host1x->dev->of_node, true); - device->dev.dma_parms = >dma_parms; dma_set_max_seg_size(>dev, UINT_MAX); Tested-by: Jon Hunter Acked-by: Jon Hunter I don't see this in -next yet? Ideally, if we don't see any issues with this we should pull this into v6.8.y stable branch because I am now seeing the warning there. Should we apply a fixes tag to this? Thanks Jon -- nvpublic
Re: [PATCH] drm/msm: Fix gen_header.py for python earlier than v3.9
Abhinav, On 08/05/2024 21:52, Jon Hunter wrote: On 08/05/2024 17:46, Abhinav Kumar wrote: On 5/8/2024 2:17 AM, Jon Hunter wrote: Building the kernel with python3 versions earlier than v3.9 fails with ... Traceback (most recent call last): File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in main() File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main parser.add_argument('--validate', action=argparse.BooleanOptionalAction) AttributeError: module 'argparse' has no attribute 'BooleanOptionalAction' The argparse attribute 'BooleanOptionalAction' is only supported for python v3.9 and later. Fix support for earlier python3 versions by explicitly defining '--validate' and '--no-validate' arguments. Signed-off-by: Jon Hunter --- drivers/gpu/drm/msm/registers/gen_header.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Thanks for your patch, I had sent something similar y'day. If you are alright with https://patchwork.freedesktop.org/patch/593057/, we can use that one. Yes that's fine with me. Any update on this? All our farm builders are unable to build either -next or mainline currently. Jon -- nvpublic
Re: [BUG] Build failure and alleged fix for next-20240523
On 24/05/2024 20:57, Abhinav Kumar wrote: Hello On 5/24/2024 12:55 PM, Paul E. McKenney wrote: Hello! I get the following allmodconfig build error on x86 in next-20240523: Traceback (most recent call last): File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in main() File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main parser.add_argument('--validate', action=argparse.BooleanOptionalAction) AttributeError: module 'argparse' has no attribute 'BooleanOptionalAction' The following patch allows the build to complete successfully: https://patchwork.kernel.org/project/dri-devel/patch/20240508091751.336654-1-jonath...@nvidia.com/#25842751 As to whether this is a proper fix, I must defer to the DRM folks on CC. Thanx, Paul Thanks for the report. I have raised a merge request for https://patchwork.freedesktop.org/patch/593057/ to make it available for the next fixes release for msm. This is also now in the mainline and so would be great to get this into both -next and mainline. Jon -- nvpublic