Nouveau - Sep 2015 - [PATCH 2/3] ltc/gf100: add flush/invalidate functions

On 3 September 2015 at 16:42, Alexandre Courbot <acourbot at nvidia.com>
wrote:
> Allow clients to manually flush and invalidate L2. This will be useful
> for Tegra systems for which we want to write instmem using the CPU.
>
> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
> ---
>  drm/nouveau/include/nvkm/subdev/ltc.h |  1 +
>  drm/nouveau/nvkm/subdev/ltc/gf100.c   | 36
+++++++++++++++++++++++++++++++++++
>  drm/nouveau/nvkm/subdev/ltc/gk104.c   |  2 ++
>  drm/nouveau/nvkm/subdev/ltc/gm107.c   |  2 ++
>  drm/nouveau/nvkm/subdev/ltc/priv.h    |  2 ++
>  5 files changed, 43 insertions(+)
>
> diff --git a/drm/nouveau/include/nvkm/subdev/ltc.h
b/drm/nouveau/include/nvkm/subdev/ltc.h
> index 5464fcf482f1..3d4dbbf9aab3 100644
> --- a/drm/nouveau/include/nvkm/subdev/ltc.h
> +++ b/drm/nouveau/include/nvkm/subdev/ltc.h
> @@ -35,5 +35,6 @@ void nvkm_ltc_flush(struct nvkm_ltc *);
>
>  int gf100_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>  int gk104_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
> +int gk20a_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>  int gm107_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>  #endif
> diff --git a/drm/nouveau/nvkm/subdev/ltc/gf100.c
b/drm/nouveau/nvkm/subdev/ltc/gf100.c
> index 45ac765b753e..42ae77533865 100644
> --- a/drm/nouveau/nvkm/subdev/ltc/gf100.c
> +++ b/drm/nouveau/nvkm/subdev/ltc/gf100.c
> @@ -122,6 +122,40 @@ gf100_ltc_intr(struct nvkm_ltc *ltc)
>         }
>  }
>
> +void
> +gf100_ltc_invalidate(struct nvkm_ltc *ltc)
> +{
> +       struct nvkm_device *device = ltc->subdev.device;
> +       s64 taken;
> +
> +       nvkm_wr32(device, 0x70004, 0x00000001);
> +       if ((taken = nvkm_msec(device, 2000,
I don't suppose you have access to information on more realistic
timeouts?  I'd like to improve all the potential 2s timeout values
across the driver in general, to avoid things hanging for a long time
when the GPU craps itself :)
> +               if ((nvkm_rd32(device, 0x70004) & 0x00000003) ==
0x00000000)
> +                       break;
> +       )) < 0)
> +               nvkm_warn(&ltc->subdev, "L2 invalidate
timeout\n");
> +
> +       if (taken > 0)
> +               nvkm_debug(&ltc->subdev, "LTC invalidate took
%lld ns\n", taken);
> +}
> +
> +void
> +gf100_ltc_flush(struct nvkm_ltc *ltc)
> +{
> +       struct nvkm_device *device = ltc->subdev.device;
> +       s64 taken;
> +
> +       nvkm_wr32(device, 0x70010, 0x00000001);
> +       if ((taken = nvkm_msec(device, 2000,
> +               if ((nvkm_rd32(device, 0x70010) & 0x00000003) ==
0x00000000)
> +                       break;
> +       )) < 0)
> +               nvkm_warn(&ltc->subdev, "L2 flush
timeout\n");
> +
> +       if (taken > 0)
> +               nvkm_debug(&ltc->subdev, "LTC flush took %lld
ns\n", taken);
> +}
> +
>  /* TODO: Figure out tag memory details and drop the over-cautious
allocation.
>   */
>  int
> @@ -215,6 +249,8 @@ gf100_ltc = {
>         .zbc = 16,
>         .zbc_clear_color = gf100_ltc_zbc_clear_color,
>         .zbc_clear_depth = gf100_ltc_zbc_clear_depth,
> +       .invalidate = gf100_ltc_invalidate,
> +       .flush = gf100_ltc_flush,
>  };
>
>  int
> diff --git a/drm/nouveau/nvkm/subdev/ltc/gk104.c
b/drm/nouveau/nvkm/subdev/ltc/gk104.c
> index 839e6b4c597b..b4f6e0034d58 100644
> --- a/drm/nouveau/nvkm/subdev/ltc/gk104.c
> +++ b/drm/nouveau/nvkm/subdev/ltc/gk104.c
> @@ -45,6 +45,8 @@ gk104_ltc = {
>         .zbc = 16,
>         .zbc_clear_color = gf100_ltc_zbc_clear_color,
>         .zbc_clear_depth = gf100_ltc_zbc_clear_depth,
> +       .invalidate = gf100_ltc_invalidate,
> +       .flush = gf100_ltc_flush,
>  };
>
>  int
> diff --git a/drm/nouveau/nvkm/subdev/ltc/gm107.c
b/drm/nouveau/nvkm/subdev/ltc/gm107.c
> index 389331bb63ba..3043bbfd7384 100644
> --- a/drm/nouveau/nvkm/subdev/ltc/gm107.c
> +++ b/drm/nouveau/nvkm/subdev/ltc/gm107.c
> @@ -138,6 +138,8 @@ gm107_ltc = {
>         .zbc = 16,
>         .zbc_clear_color = gm107_ltc_zbc_clear_color,
>         .zbc_clear_depth = gm107_ltc_zbc_clear_depth,
> +       .invalidate = gf100_ltc_invalidate,
> +       .flush = gf100_ltc_flush,
>  };
>
>  int
> diff --git a/drm/nouveau/nvkm/subdev/ltc/priv.h
b/drm/nouveau/nvkm/subdev/ltc/priv.h
> index 6f66bd03f829..4e3755b82769 100644
> --- a/drm/nouveau/nvkm/subdev/ltc/priv.h
> +++ b/drm/nouveau/nvkm/subdev/ltc/priv.h
> @@ -29,4 +29,6 @@ void gf100_ltc_cbc_clear(struct nvkm_ltc *, u32, u32);
>  void gf100_ltc_cbc_wait(struct nvkm_ltc *);
>  void gf100_ltc_zbc_clear_color(struct nvkm_ltc *, int, const u32[4]);
>  void gf100_ltc_zbc_clear_depth(struct nvkm_ltc *, int, const u32);
> +void gf100_ltc_invalidate(struct nvkm_ltc *);
> +void gf100_ltc_flush(struct nvkm_ltc *);
>  #endif
> --
> 2.5.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

On Thu, Sep 3, 2015 at 4:09 PM, Ben Skeggs <skeggsb at gmail.com>
wrote:
> On 3 September 2015 at 16:42, Alexandre Courbot <acourbot at
nvidia.com> wrote:
>> Allow clients to manually flush and invalidate L2. This will be useful
>> for Tegra systems for which we want to write instmem using the CPU.
>>
>> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
>> ---
>>  drm/nouveau/include/nvkm/subdev/ltc.h |  1 +
>>  drm/nouveau/nvkm/subdev/ltc/gf100.c   | 36
+++++++++++++++++++++++++++++++++++
>>  drm/nouveau/nvkm/subdev/ltc/gk104.c   |  2 ++
>>  drm/nouveau/nvkm/subdev/ltc/gm107.c   |  2 ++
>>  drm/nouveau/nvkm/subdev/ltc/priv.h    |  2 ++
>>  5 files changed, 43 insertions(+)
>>
>> diff --git a/drm/nouveau/include/nvkm/subdev/ltc.h
b/drm/nouveau/include/nvkm/subdev/ltc.h
>> index 5464fcf482f1..3d4dbbf9aab3 100644
>> --- a/drm/nouveau/include/nvkm/subdev/ltc.h
>> +++ b/drm/nouveau/include/nvkm/subdev/ltc.h
>> @@ -35,5 +35,6 @@ void nvkm_ltc_flush(struct nvkm_ltc *);
>>
>>  int gf100_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>>  int gk104_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>> +int gk20a_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>>  int gm107_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>>  #endif
>> diff --git a/drm/nouveau/nvkm/subdev/ltc/gf100.c
b/drm/nouveau/nvkm/subdev/ltc/gf100.c
>> index 45ac765b753e..42ae77533865 100644
>> --- a/drm/nouveau/nvkm/subdev/ltc/gf100.c
>> +++ b/drm/nouveau/nvkm/subdev/ltc/gf100.c
>> @@ -122,6 +122,40 @@ gf100_ltc_intr(struct nvkm_ltc *ltc)
>>         }
>>  }
>>
>> +void
>> +gf100_ltc_invalidate(struct nvkm_ltc *ltc)
>> +{
>> +       struct nvkm_device *device = ltc->subdev.device;
>> +       s64 taken;
>> +
>> +       nvkm_wr32(device, 0x70004, 0x00000001);
>> +       if ((taken = nvkm_msec(device, 2000,
> I don't suppose you have access to information on more realistic
> timeouts?  I'd like to improve all the potential 2s timeout values
> across the driver in general, to avoid things hanging for a long time
> when the GPU craps itself :)
The longest values I have ever seen are ~270ms (only during driver
initialization, and at low clock speeds), but anyway this should never
ever timeout. If it does, then we will have bigger issues than 2
second timeouts. :)

Btw, wouldn't it be worth to restore macros like the old nvkm_wait()
to avoid having to write this verbose code every time we need to wait
on a register value?

On 3 September 2015 at 17:13, Alexandre Courbot <gnurou at gmail.com>
wrote:
> On Thu, Sep 3, 2015 at 4:09 PM, Ben Skeggs <skeggsb at gmail.com>
wrote:
>> On 3 September 2015 at 16:42, Alexandre Courbot <acourbot at
nvidia.com> wrote:
>>> Allow clients to manually flush and invalidate L2. This will be
useful
>>> for Tegra systems for which we want to write instmem using the CPU.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
>>> ---
>>>  drm/nouveau/include/nvkm/subdev/ltc.h |  1 +
>>>  drm/nouveau/nvkm/subdev/ltc/gf100.c   | 36
+++++++++++++++++++++++++++++++++++
>>>  drm/nouveau/nvkm/subdev/ltc/gk104.c   |  2 ++
>>>  drm/nouveau/nvkm/subdev/ltc/gm107.c   |  2 ++
>>>  drm/nouveau/nvkm/subdev/ltc/priv.h    |  2 ++
>>>  5 files changed, 43 insertions(+)
>>>
>>> diff --git a/drm/nouveau/include/nvkm/subdev/ltc.h
b/drm/nouveau/include/nvkm/subdev/ltc.h
>>> index 5464fcf482f1..3d4dbbf9aab3 100644
>>> --- a/drm/nouveau/include/nvkm/subdev/ltc.h
>>> +++ b/drm/nouveau/include/nvkm/subdev/ltc.h
>>> @@ -35,5 +35,6 @@ void nvkm_ltc_flush(struct nvkm_ltc *);
>>>
>>>  int gf100_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>>>  int gk104_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>>> +int gk20a_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>>>  int gm107_ltc_new(struct nvkm_device *, int, struct nvkm_ltc **);
>>>  #endif
>>> diff --git a/drm/nouveau/nvkm/subdev/ltc/gf100.c
b/drm/nouveau/nvkm/subdev/ltc/gf100.c
>>> index 45ac765b753e..42ae77533865 100644
>>> --- a/drm/nouveau/nvkm/subdev/ltc/gf100.c
>>> +++ b/drm/nouveau/nvkm/subdev/ltc/gf100.c
>>> @@ -122,6 +122,40 @@ gf100_ltc_intr(struct nvkm_ltc *ltc)
>>>         }
>>>  }
>>>
>>> +void
>>> +gf100_ltc_invalidate(struct nvkm_ltc *ltc)
>>> +{
>>> +       struct nvkm_device *device = ltc->subdev.device;
>>> +       s64 taken;
>>> +
>>> +       nvkm_wr32(device, 0x70004, 0x00000001);
>>> +       if ((taken = nvkm_msec(device, 2000,
>> I don't suppose you have access to information on more realistic
>> timeouts?  I'd like to improve all the potential 2s timeout values
>> across the driver in general, to avoid things hanging for a long time
>> when the GPU craps itself :)
>
> The longest values I have ever seen are ~270ms (only during driver
> initialization, and at low clock speeds), but anyway this should never
> ever timeout. If it does, then we will have bigger issues than 2
> second timeouts. :)
Indeed it shouldn't, but when things get horribly messed up (as
happens occasionally :P) it's annoying and can potentially stall the
whole system until things are sorted out.  The patch is fine as-is, I
was mostly wondering if NVIDIA have data on a maximum time this should
ever take that we could use instead.
>
> Btw, wouldn't it be worth to restore macros like the old nvkm_wait()
> to avoid having to write this verbose code every time we need to wait
> on a register value?
I'm fine doing that, the reason I didn't in the first place was that
I'd planned on not having the constant 2s timeout everywhere (as
already mentioned) :)