[Intel-gfx] [PATCH 1/5] drm/i915: unify first-stage engine struct setup

[Tvrtko Ursulin]

On 13/07/16 14:16, Tvrtko Ursulin wrote:
>
> On 13/07/16 13:23, Daniel Vetter wrote:
>> On Fri, Jul 01, 2016 at 05:47:11PM +0100, Tvrtko Ursulin wrote:
>>> From: Dave Gordon <david.s.gordon at intel.com>
>>>
>>> intel_lrc.c has a table of "logical rings" (meaning engines), while
>>> intel_ringbuffer.c has separately open-coded initialisation for each
>>> engine. We can deduplicate this somewhat by using the same first-stage
>>> engine-setup function for both modes.
>>>
>>> So here we expose the function that transfers information from the
>>> static table of (all) known engines to the dev_priv->engine array of
>>> engines available on this device (adjusting the names along the way)
>>> and then embed calls to it in both the LRC and the legacy-mode setup.
>>>
>>> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_lrc.c        | 40
>>> +++++++++++++++++++++------------
>>>   drivers/gpu/drm/i915/intel_ringbuffer.c | 40
>>> +++++++++------------------------
>>>   drivers/gpu/drm/i915/intel_ringbuffer.h |  5 +++++
>>>   3 files changed, 41 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 339d8041075f..ed017f1a07a2 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -1994,8 +1994,9 @@ logical_ring_default_vfuncs(struct
>>> intel_engine_cs *engine)
>>>   }
>>>
>>>   static inline void
>>> -logical_ring_default_irqs(struct intel_engine_cs *engine, unsigned
>>> shift)
>>> +logical_ring_default_irqs(struct intel_engine_cs *engine)
>>>   {
>>> +    unsigned shift = engine->irq_shift;
>>>       engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT << shift;
>>>       engine->irq_keep_mask = GT_CONTEXT_SWITCH_INTERRUPT << shift;
>>>       init_waitqueue_head(&engine->irq_queue);
>>> @@ -2096,14 +2097,14 @@ static int logical_render_ring_init(struct
>>> intel_engine_cs *engine)
>>>       return ret;
>>>   }
>>>
>>> -static const struct logical_ring_info {
>>> +static const struct engine_info {
>>>       const char *name;
>>>       unsigned exec_id;
>>>       unsigned guc_id;
>>>       u32 mmio_base;
>>>       unsigned irq_shift;
>>>       int (*init)(struct intel_engine_cs *engine);
>>> -} logical_rings[] = {
>>> +} intel_engines[] = {
>>>       [RCS] = {
>>>           .name = "render ring",
>>>           .exec_id = I915_EXEC_RENDER,
>>> @@ -2146,20 +2147,31 @@ static const struct logical_ring_info {
>>>       },
>>>   };
>>>
>>> -static struct intel_engine_cs *
>>> -logical_ring_setup(struct drm_i915_private *dev_priv, enum
>>> intel_engine_id id)
>>> +struct intel_engine_cs *
>>> +intel_engine_setup(struct drm_i915_private *dev_priv,
>>> +           enum intel_engine_id id)
>>
>> Kerneldoc for this would be nice. Also, we now have a mess between
>> intel_lrc.c and intel_ringbuffer.c. Extracting intel_engine.c with the
>
> Yes that was already suggested by Chris and I am trybotting the
> additions today, again on his explicit request to progress this series.
>
>> shared bits or something similar, plus cleanup up all the docs would be
>> awesome as a follow up.
>
> What do you mean "all the docs"? :D
>
>>
>> With the kerneldoc added:
>>
>> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>
> Thanks, will add.

Actually the function becomes static later in the series, when the 
common stuff is moved into intel_engine_cs.c.

Can I keep the r-b without adding kernel doc for it then?

Regards,

Tvrtko