Re: [PATCH 1/2] drm/nouveau/bar/gf100: fix hang when calling ->fini() before ->init()

2017-12-05 Thread Jon Hunter

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()

2017-12-19 Thread Jon Hunter

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()

2017-12-06 Thread Jon Hunter

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

2018-01-02 Thread Jon Hunter

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

2018-01-04 Thread Jon Hunter
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

2018-08-02 Thread Jon Hunter

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

2018-09-07 Thread Jon Hunter

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

2018-09-10 Thread Jon Hunter

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

2018-09-24 Thread Jon Hunter

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

2018-11-29 Thread Jon Hunter

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

2018-11-29 Thread Jon Hunter

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"

2019-02-27 Thread Jon Hunter

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)

2019-06-18 Thread Jon Hunter

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

2019-07-09 Thread Jon Hunter

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

2019-07-09 Thread Jon Hunter

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

2019-07-24 Thread Jon Hunter


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

2019-07-09 Thread Jon Hunter


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

2020-02-24 Thread Jon Hunter
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

2020-03-13 Thread Jon Hunter
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

2020-03-25 Thread Jon Hunter


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

2020-03-25 Thread Jon Hunter


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

2020-04-27 Thread Jon Hunter
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

2020-12-01 Thread Jon Hunter

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

2020-11-30 Thread 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

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

2020-11-03 Thread 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 
---
 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

2020-11-03 Thread Jon Hunter


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

2020-11-04 Thread Jon Hunter

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

2020-11-06 Thread 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 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

2020-11-04 Thread 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);
 
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

2021-06-16 Thread Jon Hunter
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

2021-06-15 Thread Jon Hunter


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

2021-04-01 Thread Jon Hunter


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

2021-02-16 Thread Jon Hunter


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

2021-12-06 Thread Jon Hunter

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

2021-12-17 Thread Jon Hunter



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

2021-12-22 Thread Jon Hunter



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

2021-12-22 Thread Jon Hunter



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

2021-12-21 Thread 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.


Jon

--
nvpublic


Re: [PATCH v16 08/40] gpu: host1x: Add initial runtime PM and OPP support

2021-12-22 Thread Jon Hunter



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

2021-12-14 Thread Jon Hunter

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

2021-07-16 Thread Jon Hunter
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

2022-02-16 Thread Jon Hunter
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

2022-04-04 Thread Jon Hunter



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

2022-03-29 Thread Jon Hunter



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

2022-01-14 Thread Jon Hunter
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

2022-06-21 Thread Jon Hunter

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

2022-06-22 Thread Jon Hunter




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

2022-04-28 Thread Jon Hunter



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

2022-10-21 Thread Jon Hunter



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

2022-10-20 Thread Jon Hunter

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

2022-11-04 Thread Jon Hunter



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

2022-11-04 Thread Jon Hunter



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

2022-11-04 Thread Jon Hunter

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

2022-11-07 Thread Jon Hunter

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

2023-03-08 Thread Jon Hunter




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

2024-02-15 Thread Jon Hunter



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

2023-12-14 Thread Jon Hunter




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()

2024-02-01 Thread Jon Hunter



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()

2024-02-02 Thread Jon Hunter



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()

2024-02-02 Thread Jon Hunter



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()

2024-01-30 Thread Jon Hunter



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

2024-04-12 Thread Jon Hunter

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

2024-04-12 Thread Jon Hunter
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

2024-04-12 Thread Jon Hunter



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

2024-04-19 Thread Jon Hunter

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

2024-05-08 Thread Jon Hunter



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

2024-05-10 Thread Jon Hunter



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

2024-05-08 Thread Jon Hunter
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

2024-05-13 Thread Jon Hunter



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

2024-03-15 Thread Jon Hunter



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

2024-04-03 Thread Jon Hunter

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

2024-05-30 Thread Jon Hunter

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

2024-05-24 Thread Jon Hunter



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


<    1   2