On 25/01/2025 1.57, Danilo Krummrich wrote:
> Hi Zhi,
>
> On Fri, Jan 24, 2025 at 10:29:43AM -0800, Zhi Wang wrote:
>> Hi folks:
>>
>> Here is another re-spin of handling the large GSP message return.
>>
>> Besides the support of the large GSP message, kernel doc and many cleanups
>> are introduced according to the comments in the previous spin [1].
>>
>> For testing, I tried the following without any problem:
>>
>> - run Uniengine Heaven [2] on my RTX 4060 for 8 hours
>> - the GL CTS runner [3] (commandline: ./cts-runner --type-gl40) from
>> Khronos
>> - with the vGPU RFC patches [4], boot up a ubuntu VM with a vGPU on a L40
>> and run glmark2.
> Applied to drm-misc-next.
>
> Thanks a lot for this series and the iterations -- those are really good
> improvements.
>
> I fixed up the first commit "drm/nouveau: add a kernel doc to introduce the GSP
> RPC", which had some indentation issues in the bullet list and the new file was
> missing the SPDX-License-Identifier.
Hi Danilo:
Thanks so much for the efforts.
I did see checkpatch.pl reporting about SPDX-License in rst doc. But I
double checked all the
other docs. E.g. Documentation/gpu/{index.rst, drivers.rst}. It seems
none of them has the
SPDX headers. I thought it was a false warning...Will reach you in the
IRC to confirm it next
time.
> When adding documentation, please make sure to check the result after running
> the htmldocs make target.
Thanks for that. I did test the documentation generation via make
htmldocs and xmldocs
to make sure the doc generated correctly. I think you were talking about
the bullets parts
in the code where I tried to keep the length of the line < 80 but they
seems folded in the
output. Haven't figured out a approach how to fix them. I will check
your fixes.
> I also fixed up one minor indentation issue in one of the other patches.
It would be nice that you can share your checkpatch.pl command line. So
that we can align
with it. Will reach you in the IRC next time if I think something looks
like a false warning
before posting the patches.
Thanks for the review!
Z.
>
> Again, thanks for the series!
>
> - Danilo
>
>> v4:
>> - Introduce a documentation in Documentation/gpu/nouveau.rst.
>> - Fix the reference link of NVIDIA SDK headers in the document.
>> - Replace the "decoders" term with "terminology".
>> - Explain the adding offset in r535_gsp_msgq_get_entry().
>> - Replace the magic number 0x1000 with GSP_PAGE_SIZE in the re-factor.
>> - Introduce a DOC to explain the GSP message receiving flow.
>>
>> v3:
>>
>> - Add a kernel doc and chart to introduce the GSP message and denote the
>> memory layout.
>> - Add a decoder section in the kernel doc to explain the terms in the code.
>> - Refine the many confusing variable names to align with the terms in the
>> kernel doc.
>> - Introduce the continaution records in the kernel doc.
>> - Re-factor the complicated function r535_gsp_msgq_wait().
>> - Other minor cleanups.
>>
>> v2:
>>
>> - Remove the Fixes: tags as the vanilla nouveau aren't going to hit these bugs. (Danilo)
>> - Test the patchset on VK-GL-CTS. (Danilo)
>> - Remove the ambiguous empty line in PATCH 2. (Danilo)
>> - Rename the r535_gsp_msgq_wait to gsp_msgq_recv. (Danilo)
>> - Introduce a data structure to hold the params of gsp_msgq_recv(). (Danilo)
>> - Document the params and the states they are related to. (Danilo)
>>
>> [1] https://lore.kernel.org/nouveau/20241031085250.2941482-1-zhiw@xxxxxxxxxx/
>> [2] https://benchmark.unigine.com/heaven
>> [3] https://github.com/KhronosGroup/VK-GL-CTS
>> [4] https://lore.kernel.org/kvm/20240922124951.1946072-1-zhiw@xxxxxxxxxx/T/#t
>>
>>
>> Zhi Wang (15):
>> drm/nouveau: add a kernel doc to introduce the GSP RPC
>> drm/nouveau: rename "repc" to "gsp_rpc_len" on the GSP message recv
>> path
>> drm/nouveau: rename "argv" to what it represents on the GSP message
>> send path
>> drm/nouveau: remove unused param repc in *rm_alloc_push()
>> drm/nouveau: rename "argv" to what it represents in *rm_{alloc,
>> ctrl}_*()
>> drm/nouveau: rename "argc" to what it represents in GSP RPC routines
>> drm/nouveau: fix the broken marco GSP_MSG_MAX_SIZE
>> drm/nouveau: remove the magic number in r535_gsp_rpc_push()
>> drm/nouveau: refine the variable names in r535_gsp_rpc_push()
>> drm/nouveau: refine the variable names in r535_gsp_msg_recv()
>> drm/nouveau: rename the variable "cmd" to "msg" in r535_gsp_cmdq_{get,
>> push}()
>> drm/nouveau: factor out r535_gsp_msgq_peek()
>> drm/nouveau: factor out r535_gsp_msgq_recv_one_elem()
>> drm/nouveau: support handling the return of large GSP message
>> drm/nouveau: consume the return of large GSP message
>>
>> Documentation/gpu/drivers.rst | 1 +
>> Documentation/gpu/nouveau.rst | 27 +
>> .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 8 +-
>> .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 526 +++++++++++++-----
>> 4 files changed, 409 insertions(+), 153 deletions(-)
>> create mode 100644 Documentation/gpu/nouveau.rst
>>
>> --
>> 2.34.1
>>
