Re: [PATCH 04/15] drm/i915: Add GuC-related header files

Daniel Vetter
2015-06-24T09:34:17Z

On Wed, Jun 24, 2015 at 08:41:02AM +0100, Dave Gordon wrote:
> On 15/06/15 21:20, Chris Wilson wrote:
> >> + struct ida ctx_ids;
> >> + uint32_t log_flags;
> >> + int db_cacheline;
> >> + DECLARE_BITMAP(doorbell_bitmap, I915_MAX_DOORBELLS);
> >> +
> >> + /* Action status & statistics */
> >> + uint64_t action_count; /* Total commands issued */
> >> + uint32_t action_cmd; /* Last command word */
> >> + uint32_t action_status; /* Last return status */
> >> + uint32_t action_fail; /* Total number of failures */
> >> + int32_t action_err; /* Last error code */
> >
> > Any group of prefix_ immediately raises the question of "why isn't this
> > a struct?"
> > -Chris
>
> Not really worth making and naming a struct. There's only one instance
> of this whole thing; the code that updates these touches them
> individually, and the debugfs code that prints them can't really make
> use of them collectively either.

We have a lot of single-instance structs all over the place to group
related data around. It imo does help a lot, but yeah might be on the
fence here.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation