| Message ID | 1538397105-19581-3-git-send-email-smasetty@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: Hook up the DRM gpu scheduler | expand |
On Mon, Oct 01, 2018 at 06:01:34PM +0530, Sharat Masetty wrote: > When the scheduler comes into picture, the msm_gpu_submit() will be > called from the scheduler worker thread. So save the necessary information > into msm job structure for use at the actual GPU submission time. This > also simplifies the msm_gpu_submit() API. When I read this, I immediately thought you were changing the format of the submit ioctl struct, but really you are just changing the parameters of the functions - this isn't an API by definition because it isn't an interface to anything else. Words like API get people all worked up - I would recommend rewording this to be less scary / more accurate. > Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> > --- > drivers/gpu/drm/msm/msm_gem.h | 1 + > drivers/gpu/drm/msm/msm_gem_submit.c | 8 +++++--- > drivers/gpu/drm/msm/msm_gpu.c | 8 ++++---- > drivers/gpu/drm/msm/msm_gpu.h | 3 +-- > 4 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h > index 287f974..289abe5 100644 > --- a/drivers/gpu/drm/msm/msm_gem.h > +++ b/drivers/gpu/drm/msm/msm_gem.h > @@ -137,6 +137,7 @@ enum msm_gem_lock { > */ > struct msm_gem_submit { > struct drm_device *dev; > + struct msm_file_private *ctx; > struct msm_gpu *gpu; > struct list_head node; /* node in ring submit list */ > struct list_head bo_list; > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c > index 00e6347..14906b9 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -32,7 +32,7 @@ > > static struct msm_gem_submit *submit_create(struct drm_device *dev, > struct msm_gpu *gpu, struct msm_gpu_submitqueue *queue, > - uint32_t nr_bos, uint32_t nr_cmds) > + uint32_t nr_bos, uint32_t nr_cmds, struct msm_file_private *ctx) submit_create() is a catch-22. If I saw all that code inline I would probably want it to be a sub function too, but on the other hand we're steadily adding new parameters to it and adding new lines of code that the calling function doesn't need to translate. > { > struct msm_gem_submit *submit; > uint64_t sz = sizeof(*submit) + ((u64)nr_bos * sizeof(submit->bos[0])) + > @@ -47,6 +47,7 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, > > submit->dev = dev; > submit->gpu = gpu; > + submit->ctx = ctx; > submit->fence = NULL; > submit->out_fence_id = -1; > submit->pid = get_pid(task_pid(current)); > @@ -474,7 +475,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, > } > } > > - submit = submit_create(dev, gpu, queue, args->nr_bos, args->nr_cmds); > + submit = submit_create(dev, gpu, queue, args->nr_bos, > + args->nr_cmds, ctx); Like maybe we can compromise and pass in args directly to submit_create to help offset the new parameters. > if (!submit) { > ret = -ENOMEM; > goto out_unlock; > @@ -560,7 +562,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, > > submit->nr_cmds = i; > > - msm_gpu_submit(gpu, submit, ctx); > + msm_gpu_submit(submit); > if (IS_ERR(submit->fence)) { > ret = PTR_ERR(submit->fence); > submit->fence = NULL; > diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c > index eb67172..5f9cd85 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.c > +++ b/drivers/gpu/drm/msm/msm_gpu.c > @@ -602,9 +602,9 @@ void msm_gpu_retire(struct msm_gpu *gpu) > } > > /* add bo's to gpu's ring, and kick gpu: */ > -struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu, > - struct msm_gem_submit *submit, struct msm_file_private *ctx) > +struct dma_fence *msm_gpu_submit(struct msm_gem_submit *submit) > { > + struct msm_gpu *gpu = submit->gpu; > struct drm_device *dev = gpu->dev; > struct msm_drm_private *priv = dev->dev_private; > struct msm_ringbuffer *ring = submit->ring; > @@ -648,8 +648,8 @@ struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu, > msm_gem_move_to_active(&msm_obj->base, gpu, false, submit->fence); > } > > - gpu->funcs->submit(gpu, submit, ctx); > - priv->lastctx = ctx; > + gpu->funcs->submit(gpu, submit, submit->ctx); > + priv->lastctx = submit->ctx; > > hangcheck_timer_reset(gpu); > > diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h > index b562b25..dd55979 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.h > +++ b/drivers/gpu/drm/msm/msm_gpu.h > @@ -235,8 +235,7 @@ int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint32_t *activetime, > uint32_t *totaltime, uint32_t ncntrs, uint32_t *cntrs); > > void msm_gpu_retire(struct msm_gpu *gpu); > -struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu, > - struct msm_gem_submit *submit, struct msm_file_private *ctx); > +struct dma_fence *msm_gpu_submit(struct msm_gem_submit *submit); > > int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, > struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs, > -- > 1.9.1 >
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 287f974..289abe5 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -137,6 +137,7 @@ enum msm_gem_lock { */ struct msm_gem_submit { struct drm_device *dev; + struct msm_file_private *ctx; struct msm_gpu *gpu; struct list_head node; /* node in ring submit list */ struct list_head bo_list; diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 00e6347..14906b9 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -32,7 +32,7 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, struct msm_gpu *gpu, struct msm_gpu_submitqueue *queue, - uint32_t nr_bos, uint32_t nr_cmds) + uint32_t nr_bos, uint32_t nr_cmds, struct msm_file_private *ctx) { struct msm_gem_submit *submit; uint64_t sz = sizeof(*submit) + ((u64)nr_bos * sizeof(submit->bos[0])) + @@ -47,6 +47,7 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, submit->dev = dev; submit->gpu = gpu; + submit->ctx = ctx; submit->fence = NULL; submit->out_fence_id = -1; submit->pid = get_pid(task_pid(current)); @@ -474,7 +475,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, } } - submit = submit_create(dev, gpu, queue, args->nr_bos, args->nr_cmds); + submit = submit_create(dev, gpu, queue, args->nr_bos, + args->nr_cmds, ctx); if (!submit) { ret = -ENOMEM; goto out_unlock; @@ -560,7 +562,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, submit->nr_cmds = i; - msm_gpu_submit(gpu, submit, ctx); + msm_gpu_submit(submit); if (IS_ERR(submit->fence)) { ret = PTR_ERR(submit->fence); submit->fence = NULL; diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index eb67172..5f9cd85 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -602,9 +602,9 @@ void msm_gpu_retire(struct msm_gpu *gpu) } /* add bo's to gpu's ring, and kick gpu: */ -struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu, - struct msm_gem_submit *submit, struct msm_file_private *ctx) +struct dma_fence *msm_gpu_submit(struct msm_gem_submit *submit) { + struct msm_gpu *gpu = submit->gpu; struct drm_device *dev = gpu->dev; struct msm_drm_private *priv = dev->dev_private; struct msm_ringbuffer *ring = submit->ring; @@ -648,8 +648,8 @@ struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu, msm_gem_move_to_active(&msm_obj->base, gpu, false, submit->fence); } - gpu->funcs->submit(gpu, submit, ctx); - priv->lastctx = ctx; + gpu->funcs->submit(gpu, submit, submit->ctx); + priv->lastctx = submit->ctx; hangcheck_timer_reset(gpu); diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index b562b25..dd55979 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -235,8 +235,7 @@ int msm_gpu_perfcntr_sample(struct msm_gpu *gpu, uint32_t *activetime, uint32_t *totaltime, uint32_t ncntrs, uint32_t *cntrs); void msm_gpu_retire(struct msm_gpu *gpu); -struct dma_fence *msm_gpu_submit(struct msm_gpu *gpu, - struct msm_gem_submit *submit, struct msm_file_private *ctx); +struct dma_fence *msm_gpu_submit(struct msm_gem_submit *submit); int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs,
When the scheduler comes into picture, the msm_gpu_submit() will be called from the scheduler worker thread. So save the necessary information into msm job structure for use at the actual GPU submission time. This also simplifies the msm_gpu_submit() API. Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> --- drivers/gpu/drm/msm/msm_gem.h | 1 + drivers/gpu/drm/msm/msm_gem_submit.c | 8 +++++--- drivers/gpu/drm/msm/msm_gpu.c | 8 ++++---- drivers/gpu/drm/msm/msm_gpu.h | 3 +-- 4 files changed, 11 insertions(+), 9 deletions(-)