Re: [PATCH 004/156] drm/nouveau: pass drm to nv50_dmac_create(), rather than device+disp

[Philipp Stanner <pstanner@redhat.com>]

On Wed, 2024-04-17 at 09:37 +1000, Ben Skeggs wrote:
> - zero reason to do otherwise

to do _what_ otherwise? that refers to the title, it seems. But one
could describe why it was ever done that way (older architecture? bug?
mistake?) in the first place.

The commit messages in this entire series are extremely short and only
really comprehensible by the author.

This example here especially.

The messages should first briefly introduce what the current state is,
why that's a problem and what the commit does about it.

P.

> 
> Signed-off-by: Ben Skeggs <bskeggs@nvidia.com>
> ---
>  drivers/gpu/drm/nouveau/dispnv50/base507c.c | 3 +--
>  drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c     | 7 ++++---
>  drivers/gpu/drm/nouveau/dispnv50/disp.h     | 2 +-
>  drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +-
>  drivers/gpu/drm/nouveau/dispnv50/wimmc37b.c | 3 +--
>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +-
>  7 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/base507c.c
> b/drivers/gpu/drm/nouveau/dispnv50/base507c.c
> index 70c62b861276..e36a473f2075 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/base507c.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/base507c.c
> @@ -307,7 +307,6 @@ base507c_new_(const struct nv50_wndw_func *func,
> const u32 *format,
>         struct nvif_disp_chan_v0 args = {
>                 .id = head,
>         };
> -       struct nouveau_display *disp = nouveau_display(drm->dev);
>         struct nv50_disp *disp50 = nv50_disp(drm->dev);
>         struct nv50_wndw *wndw;
>         int ret;
> @@ -318,7 +317,7 @@ base507c_new_(const struct nv50_wndw_func *func,
> const u32 *format,
>         if (*pwndw = wndw, ret)
>                 return ret;
>  
> -       ret = nv50_dmac_create(&drm->client.device, &disp-
> >disp.object,
> +       ret = nv50_dmac_create(drm,
>                                &oclass, head, &args, sizeof(args),
>                                disp50->sync->offset, &wndw->wndw);
>         if (ret) {
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/core507d.c
> b/drivers/gpu/drm/nouveau/dispnv50/core507d.c
> index e5bb5ca950c8..85845e3dc7ba 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c
> @@ -166,7 +166,7 @@ core507d_new_(const struct nv50_core_func *func,
> struct nouveau_drm *drm,
>                 return -ENOMEM;
>         core->func = func;
>  
> -       ret = nv50_dmac_create(&drm->client.device, &disp->disp-
> >object,
> +       ret = nv50_dmac_create(drm,
>                                &oclass, 0, &args, sizeof(args),
>                                disp->sync->offset, &core->chan);
>         if (ret) {
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 0c3d88ad0b0e..f8ca57cc7af6 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -226,11 +226,12 @@ static int nv50_dmac_vram_pushbuf = -1;
>  module_param_named(kms_vram_pushbuf, nv50_dmac_vram_pushbuf, int,
> 0400);
>  
>  int
> -nv50_dmac_create(struct nvif_device *device, struct nvif_object
> *disp,
> +nv50_dmac_create(struct nouveau_drm *drm,
>                  const s32 *oclass, u8 head, void *data, u32 size,
> s64 syncbuf,
>                  struct nv50_dmac *dmac)
>  {
> -       struct nouveau_cli *cli = (void *)device->object.client;
> +       struct nvif_device *device = &drm->device;
> +       struct nvif_object *disp = &drm->display->disp.object;
>         struct nvif_disp_chan_v0 *args = data;
>         u8 type = NVIF_MEM_COHERENT;
>         int ret;
> @@ -250,7 +251,7 @@ nv50_dmac_create(struct nvif_device *device,
> struct nvif_object *disp,
>             (nv50_dmac_vram_pushbuf < 0 && device->info.family ==
> NV_DEVICE_INFO_V0_PASCAL))
>                 type |= NVIF_MEM_VRAM;
>  
> -       ret = nvif_mem_ctor_map(&cli->mmu, "kmsChanPush", type,
> 0x1000,
> +       ret = nvif_mem_ctor_map(&drm->mmu, "kmsChanPush", type,
> 0x1000,
>                                 &dmac->_push.mem);
>         if (ret)
>                 return ret;
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.h
> b/drivers/gpu/drm/nouveau/dispnv50/disp.h
> index 5508a7cfd492..da3add95f354 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.h
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.h
> @@ -95,7 +95,7 @@ struct nv50_outp_atom {
>         } set, clr;
>  };
>  
> -int nv50_dmac_create(struct nvif_device *device, struct nvif_object
> *disp,
> +int nv50_dmac_create(struct nouveau_drm *,
>                      const s32 *oclass, u8 head, void *data, u32
> size,
>                      s64 syncbuf, struct nv50_dmac *dmac);
>  void nv50_dmac_destroy(struct nv50_dmac *);
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
> b/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
> index 797c1e4e0eaa..73fcfb27c32c 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
> @@ -159,7 +159,7 @@ ovly507e_new_(const struct nv50_wndw_func *func,
> const u32 *format,
>         if (*pwndw = wndw, ret)
>                 return ret;
>  
> -       ret = nv50_dmac_create(&drm->client.device, &disp->disp-
> >object,
> +       ret = nv50_dmac_create(drm,
>                                &oclass, 0, &args, sizeof(args),
>                                disp->sync->offset, &wndw->wndw);
>         if (ret) {
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wimmc37b.c
> b/drivers/gpu/drm/nouveau/dispnv50/wimmc37b.c
> index ee76b091d4ef..8cb5b79bacbf 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wimmc37b.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wimmc37b.c
> @@ -71,10 +71,9 @@ wimmc37b_init_(const struct nv50_wimm_func *func,
> struct nouveau_drm *drm,
>         struct nvif_disp_chan_v0 args = {
>                 .id = wndw->id,
>         };
> -       struct nv50_disp *disp = nv50_disp(drm->dev);
>         int ret;
>  
> -       ret = nv50_dmac_create(&drm->client.device, &disp->disp-
> >object,
> +       ret = nv50_dmac_create(drm,
>                                &oclass, 0, &args, sizeof(args), -1,
>                                &wndw->wimm);
>         if (ret) {
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
> b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
> index b3deea5aca58..caf40977f455 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
> @@ -363,7 +363,7 @@ wndwc37e_new_(const struct nv50_wndw_func *func,
> struct nouveau_drm *drm,
>         if (*pwndw = wndw, ret)
>                 return ret;
>  
> -       ret = nv50_dmac_create(&drm->client.device, &disp->disp-
> >object,
> +       ret = nv50_dmac_create(drm,
>                                &oclass, 0, &args, sizeof(args),
>                                disp->sync->offset, &wndw->wndw);
>         if (ret) {