Re: [PATCH] drm/panfrost: Queue jobs on the hardware

From: Rob Herring
Date: Mon Aug 19 2019 - 13:02:30 EST


On Mon, Aug 19, 2019 at 11:58 AM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Fri, Aug 16, 2019 at 4:31 AM Steven Price <steven.price@xxxxxxx> wrote:
> >
> > The hardware has a set of '_NEXT' registers that can hold a second job
> > while the first is executing. Make use of these registers to enqueue a
> > second job per slot.
> >
> > Signed-off-by: Steven Price <steven.price@xxxxxxx>
> > ---
> > Note that this is based on top of Rob Herring's "per FD address space"
> > patch[1].
> >
> > [1] https://marc.info/?i=20190813150115.30338-1-robh%20()%20kernel%20!%20org
> >
> > drivers/gpu/drm/panfrost/panfrost_device.h | 4 +-
> > drivers/gpu/drm/panfrost/panfrost_job.c | 76 ++++++++++++++++++----
> > drivers/gpu/drm/panfrost/panfrost_mmu.c | 2 +-
> > 3 files changed, 67 insertions(+), 15 deletions(-)
>
> LGTM, but I'll give Tomeu a chance to comment.

Though checkpatch reports some style nits:

-:46: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written
"!pfdev->jobs[slot][0]"
#46: FILE: drivers/gpu/drm/panfrost/panfrost_job.c:143:
+ if (pfdev->jobs[slot][0] == NULL)

-:48: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written
"!pfdev->jobs[slot][1]"
#48: FILE: drivers/gpu/drm/panfrost/panfrost_job.c:145:
+ if (pfdev->jobs[slot][1] == NULL)

-:53: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#53: FILE: drivers/gpu/drm/panfrost/panfrost_job.c:150:
+static struct panfrost_job *panfrost_dequeue_job(

-:67: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written
"!pfdev->jobs[slot][0]"
#67: FILE: drivers/gpu/drm/panfrost/panfrost_job.c:164:
+ if (pfdev->jobs[slot][0] == NULL) {

-:71: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written
"pfdev->jobs[slot][1]"
#71: FILE: drivers/gpu/drm/panfrost/panfrost_job.c:168:
+ WARN_ON(pfdev->jobs[slot][1] != NULL);

-:160: ERROR:SPACING: space prohibited before that '--' (ctx:WxO)
#160: FILE: drivers/gpu/drm/panfrost/panfrost_job.c:497:
+ jobs --;
^

-:165: ERROR:SPACING: space required one side of that '--' (ctx:WxW)
#165: FILE: drivers/gpu/drm/panfrost/panfrost_job.c:500:
+ while (jobs -- > active) {
^

-:204: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#204: FILE: drivers/gpu/drm/panfrost/panfrost_mmu.c:150:
+ WARN_ON(en >= NUM_JOB_SLOTS*2);
^