[PATCH v3 1/1] drm/panfrost: Add support for devcoredump
Alyssa Rosenzweig
alyssa at collabora.com
Wed Jun 22 14:16:32 UTC 2022
> Sorry about this blunder.
>
> >> + slot = panfrost_job_get_slot(job);
> >> + slot = slot ? slot : 0;
> >
> >`slot = slot ? slot : 0` is a no-op. Delete the line.
>
> I think what I meant here was 'slot = (slot >= 0) ? slot : 0;' but for some
> reason I blundered again. The point of this was ensuring the slot value wouldn't
> end up wrapping about the maximum unsigned integer value when using it as an
> array offset, in the off-chance that panfrost_job_get_slot() ever returned a
> negative value.
>
> In v4 I've instead rewritten this as a sanity check:
>
> WARN_ON(slot < 0);
No, this doesn't make sense. There at most 3 job slots -- 0, 1, and 2.
> Although perhaps in the future panfrost_job_get_slot should return an unsigned
> integer instead?
Sure. Kernel style doesn't seem big on unsigned, if this were
mesa it would return unsigned. Returning u8 or u32 seems reasonable at
any rate.
> >As a general note, I'd appreciate breaking out the panfrost_regs.h
> >changes into a separate patch, as they are a logically separate clean
> >up to make room for this patch. Thanks.
>
> Done in v4.
Thanks!
Alyssa
More information about the dri-devel
mailing list