Re: [PATCH] drm/msm: Optimize GPU crashstate capture read path

[Jordan Crouse <jcrouse-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On Fri, Nov 02, 2018 at 03:12:51PM +0530, Sharat Masetty wrote:
> Thanks for the comments Jordan -
> 
> On 11/1/2018 8:34 PM, Jordan Crouse wrote:
> >On Thu, Nov 01, 2018 at 02:05:41PM +0530, Sharat Masetty wrote:
> >>When the userspace tries to read the crashstate dump, the read side
> >>implementation in the driver currently ascii85 encodes all the binary
> >>buffers and it does this each time the read system call is called.
> >>A userspace tool like cat typically does a page by page read and the
> >>number of read calls depends on the size of the data captured by the
> >>driver. This is certainly not desirable and does not scale well with
> >>large captures.
> >>
> >>This patch starts off with moving the encoding part to the capture time,
> >>that is when the GPU tries to recover after a hang. Now we only encode
> >>once per buffer object and not per page read. With this there is an
> >>immediate >10X speed improvement in crashstate save time.
> >>
> >>The flip side however is that the GPU recovery time increases and this
> >>negatively impacts the user experience, so this is handled by moving the
> >>encoding part to a worker thread and only register with the dev_coredump
> >>once the encoding of the buffers is complete.
> >
> >Does your tree have https://patchwork.freedesktop.org/patch/257181/ ? That will
> >help with a lot of the problems you have described.
> The issue is that even with small sized files like ~5MB or so, the
> capture time is huge and sometime does not survive the dev_coredump
> timeout of 5 minutes. With this change however I am able to dump
> large file sizes like 60MB or so in reasonable amount of time (~50
> seconds).
> 
> As for the patch that you shared, I am thinking that we should drop
> it and instead go with this or whatever we agree on. The patch loses
> stuff like IB2's which might be important while debugging hang
> issues.

You won't lose IB2s if you mark them as MSM_SUBMIT_CMD_IB_TARGET_BUF and Rob is
working on some new changes for userspace to identify buffers as "dumpable"
which will coincide with the patch I posted.

Thats not to say that your patch isn't appropriate - there is good stuff here
but even with it in place we probably shouldn't be generating 60MB dumps unless
we're going for some sort of replay scenario (which we could enabled with an
appropriate debugfs flag).

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno