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

Open Source and information security mailing list archives

Message-ID: <2dbb2b86-c409-1f58-275d-bec054da4dd5@arm.com>
Date:   Wed, 21 Aug 2019 09:57:54 +0100
From:   Steven Price <steven.price@....com>
To:     Rob Herring <robh@...nel.org>
Cc:     dri-devel <dri-devel@...ts.freedesktop.org>,
        Tomeu Vizoso <tomeu.vizoso@...labora.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/panfrost: Queue jobs on the hardware

On 19/08/2019 18:02, Rob Herring wrote:
> On Mon, Aug 19, 2019 at 11:58 AM Rob Herring <robh@...nel.org> wrote:
>>
>> On Fri, Aug 16, 2019 at 4:31 AM Steven Price <steven.price@....com> 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@....com>
>>> ---
>>> 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:

Gah! Sorry - I probably should have pushed this out as an RFC anyway. My
DDK-on-Panfrost investigation showed a decent performance improvement,
but I hadn't actually tested with the Mesa driver. And as Tomeu has
discovered that it actually slows down I guess we need to investigate
that before merging.

Steve

> -: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);
>                                            ^
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives