On Tue, Feb 18, 2020 at 01:00:21PM -0800, Rob Clark wrote: > From: Rob Clark <robdclark@xxxxxxxxxxxx> > > Also log buffers with the DUMP flag set, to ensure we capture all useful > cmdstream in crashdump state with modern mesa. > > Otherwise we miss out on the contents of "state object" cmdstream > buffers. One nit, but with that: Reviewed-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> > Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx> > --- > drivers/gpu/drm/msm/msm_gem.h | 10 ++++++++++ > drivers/gpu/drm/msm/msm_gpu.c | 28 +++++++++++++++++++++++----- > drivers/gpu/drm/msm/msm_rd.c | 8 +------- > 3 files changed, 34 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h > index 9e0953c2b7ce..22b4ccd7bb28 100644 > --- a/drivers/gpu/drm/msm/msm_gem.h > +++ b/drivers/gpu/drm/msm/msm_gem.h > @@ -160,4 +160,14 @@ struct msm_gem_submit { > } bos[0]; > }; > > +/* helper to determine of a buffer in submit should be dumped, used for both > + * devcoredump and debugfs cmdstream dumping: > + */ > +static bool Static inline? Surprised you didn't get an unused warning or two. > +should_dump(struct msm_gem_submit *submit, int idx) > +{ > + extern bool rd_full; > + return rd_full || (submit->bos[idx].flags & MSM_SUBMIT_BO_DUMP); > +} > + > #endif /* __MSM_GEM_H__ */ > diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c > index 18f3a5c53ffb..615c5cda5389 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.c > +++ b/drivers/gpu/drm/msm/msm_gpu.c > @@ -355,16 +355,34 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu, > state->cmd = kstrdup(cmd, GFP_KERNEL); > > if (submit) { > - int i; > - > - state->bos = kcalloc(submit->nr_cmds, > + int i, nr = 0; > + > + /* count # of buffers to dump: */ > + for (i = 0; i < submit->nr_bos; i++) > + if (should_dump(submit, i)) > + nr++; > + /* always dump cmd bo's, but don't double count them: */ > + for (i = 0; i < submit->nr_cmds; i++) > + if (!should_dump(submit, submit->cmd[i].idx)) > + nr++; > + > + state->bos = kcalloc(nr, > sizeof(struct msm_gpu_state_bo), GFP_KERNEL); > > + for (i = 0; i < submit->nr_bos; i++) { > + if (should_dump(submit, i)) { > + msm_gpu_crashstate_get_bo(state, submit->bos[i].obj, > + submit->bos[i].iova, submit->bos[i].flags); > + } > + } > + > for (i = 0; state->bos && i < submit->nr_cmds; i++) { > int idx = submit->cmd[i].idx; > > - msm_gpu_crashstate_get_bo(state, submit->bos[idx].obj, > - submit->bos[idx].iova, submit->bos[idx].flags); > + if (!should_dump(submit, submit->cmd[i].idx)) { > + msm_gpu_crashstate_get_bo(state, submit->bos[idx].obj, > + submit->bos[idx].iova, submit->bos[idx].flags); > + } > } > } > > diff --git a/drivers/gpu/drm/msm/msm_rd.c b/drivers/gpu/drm/msm/msm_rd.c > index af7ceb246c7c..732f65df5c4f 100644 > --- a/drivers/gpu/drm/msm/msm_rd.c > +++ b/drivers/gpu/drm/msm/msm_rd.c > @@ -43,7 +43,7 @@ > #include "msm_gpu.h" > #include "msm_gem.h" > > -static bool rd_full = false; > +bool rd_full = false; > MODULE_PARM_DESC(rd_full, "If true, $debugfs/.../rd will snapshot all buffer contents"); > module_param_named(rd_full, rd_full, bool, 0600); > > @@ -336,12 +336,6 @@ static void snapshot_buf(struct msm_rd_state *rd, > msm_gem_put_vaddr(&obj->base); > } > > -static bool > -should_dump(struct msm_gem_submit *submit, int idx) > -{ > - return rd_full || (submit->bos[idx].flags & MSM_SUBMIT_BO_DUMP); > -} > - > /* called under struct_mutex */ > void msm_rd_dump_submit(struct msm_rd_state *rd, struct msm_gem_submit *submit, > const char *fmt, ...) > -- > 2.24.1 > > _______________________________________________ > Freedreno mailing list > Freedreno@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel