[Intel-gfx] [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality

Kamble, Sagar A sagar.a.kamble at intel.com
Mon Sep 18 06:23:12 UTC 2017



On 9/18/2017 1:00 AM, Michal Wajdeczko wrote:
> On Sun, 17 Sep 2017 14:17:25 +0200, Sagar Arun Kamble 
> <sagar.a.kamble at intel.com> wrote:
>
>> Create intel_guc.c and move guc communication init functionality from
>> intel_uc.c. Prepared new initialization function intel_guc_init_early.
>> Moved below functions to intel_guc.c.
>> 1. intel_guc_send_nop
>> 2. gen8_guc_raise_irq
>> And below functions to intel_guc.h.
>> 1. intel_guc_send
>> 2. intel_guc_notify
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Michał Winiarski <michal.winiarski at intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> ---
>>  drivers/gpu/drm/i915/Makefile              |  1 +
>>  drivers/gpu/drm/i915/i915_guc_submission.c |  1 +
>>  drivers/gpu/drm/i915/intel_guc.c           | 47 
>> ++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_guc.h           | 43 
>> +++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_guc_ct.c        |  1 +
>>  drivers/gpu/drm/i915/intel_guc_log.c       |  1 +
>>  drivers/gpu/drm/i915/intel_huc.c           |  1 +
>>  drivers/gpu/drm/i915/intel_uc.c            | 22 ++------------
>>  drivers/gpu/drm/i915/intel_uc.h            | 11 -------
>>  9 files changed, 97 insertions(+), 31 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/intel_guc.c
>>  create mode 100644 drivers/gpu/drm/i915/intel_guc.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile 
>> b/drivers/gpu/drm/i915/Makefile
>> index 1cb8059..e13fc19 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -59,6 +59,7 @@ i915-y += i915_cmd_parser.o \
>> # general-purpose microcontroller (GuC) support
>>  i915-y += intel_uc.o \
>> +      intel_guc.o \
>>        intel_guc_ct.o \
>>        intel_guc_log.o \
>>        intel_guc_loader.o \
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index 3f9d227..f37cd47 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/circ_buf.h>
>>  #include "i915_drv.h"
>>  #include "intel_uc.h"
>> +#include "intel_guc.h"
>
> Please reorder your patches and make sure that no
> explicit #include "intel_guc.h" are needed.
Sure. Will update.
>
>> #include <trace/events/dma_fence.h>
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c 
>> b/drivers/gpu/drm/i915/intel_guc.c
>> new file mode 100644
>> index 0000000..0c62cc2
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * Copyright © 2017 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial 
>> portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + */
>> +
>> +#include "i915_drv.h"
>> +
>> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len)
>> +{
>> +    WARN(1, "Unexpected send: action=%#x\n", *action);
>> +    return -ENODEV;
>> +}
>> +
>
> hmm, you're moving send functions in patch 002/010
I think I should squash these patches together as I don't see clear 
separation.
Will prepare new patch to take out any delta refactoring done.
>
>> +static void gen8_guc_raise_irq(struct intel_guc *guc)
>> +{
>> +    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> +
>> +    I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
>> +}
>> +
>> +void intel_guc_init_early(struct intel_guc *guc)
>> +{
>> +    intel_guc_ct_init_early(&guc->ct);
>> +
>> +    mutex_init(&guc->send_mutex);
>> +    guc->send = intel_guc_send_nop;
>> +    guc->notify = gen8_guc_raise_irq;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
>> b/drivers/gpu/drm/i915/intel_guc.h
>> new file mode 100644
>> index 0000000..76b7113
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -0,0 +1,43 @@
>> +/*
>> + * Copyright © 2017 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial 
>> portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + */
>> +#ifndef _INTEL_GUC_H_
>> +#define _INTEL_GUC_H_
>> +
>
> hmm, starting with empty header does not look good.
>
>> +static inline int intel_guc_send(struct intel_guc *guc,
>> +                 const u32 *action,
>> +                 u32 len)
>> +{
>> +    return guc->send(guc, action, len);
>> +}
>> +
>
> hmm, you're moving send functions in patch 002/010
Yes. Will update.
>
>> +static inline void intel_guc_notify(struct intel_guc *guc)
>> +{
>> +    guc->notify(guc);
>> +}
>> +
>> +/* intel_guc.c */
>> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len);
>> +void intel_guc_init_early(struct intel_guc *guc);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
>> b/drivers/gpu/drm/i915/intel_guc_ct.c
>> index c4cbec1..87dd12d 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>> @@ -23,6 +23,7 @@
>> #include "i915_drv.h"
>>  #include "intel_guc_ct.h"
>> +#include "intel_guc.h"
>
> please try to avoid this
Yes. Will update at all places.
>
>> enum { CTB_SEND = 0, CTB_RECV = 1 };
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
>> b/drivers/gpu/drm/i915/intel_guc_log.c
>> index 16d3b87..5c7e0c2 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/debugfs.h>
>>  #include <linux/relay.h>
>>  #include "i915_drv.h"
>> +#include "intel_guc.h"
>
> please try to avoid this
>
>> static void guc_log_capture_logs(struct intel_guc *guc);
>> diff --git a/drivers/gpu/drm/i915/intel_huc.c 
>> b/drivers/gpu/drm/i915/intel_huc.c
>> index 6145fa0..b81c6af 100644
>> --- a/drivers/gpu/drm/i915/intel_huc.c
>> +++ b/drivers/gpu/drm/i915/intel_huc.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/firmware.h>
>>  #include "i915_drv.h"
>>  #include "intel_uc.h"
>> +#include "intel_guc.h"
>
> please try to avoid this
>
>> /**
>>   * DOC: HuC Firmware
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 0178ba4..9ff5c97 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -24,6 +24,7 @@
>> #include "i915_drv.h"
>>  #include "intel_uc.h"
>> +#include "intel_guc.h"
>
> please try to avoid this
>
>>  #include <linux/firmware.h>
>> /* Cleans up uC firmware by releasing the firmware GEM obj.
>> @@ -94,22 +95,9 @@ void intel_uc_sanitize_options(struct 
>> drm_i915_private *dev_priv)
>>          i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
>>  }
>> -static void gen8_guc_raise_irq(struct intel_guc *guc)
>> -{
>> -    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> -
>> -    I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
>> -}
>> -
>>  void intel_uc_init_early(struct drm_i915_private *dev_priv)
>>  {
>> -    struct intel_guc *guc = &dev_priv->guc;
>> -
>> -    intel_guc_ct_init_early(&guc->ct);
>> -
>> -    mutex_init(&guc->send_mutex);
>> -    guc->send = intel_guc_send_nop;
>> -    guc->notify = gen8_guc_raise_irq;
>> +    intel_guc_init_early(&dev_priv->guc);
>>  }
>> static void fetch_uc_fw(struct drm_i915_private *dev_priv,
>> @@ -459,12 +447,6 @@ void intel_uc_fini_hw(struct drm_i915_private 
>> *dev_priv)
>>      i915_ggtt_disable_guc(dev_priv);
>>  }
>> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len)
>> -{
>> -    WARN(1, "Unexpected send: action=%#x\n", *action);
>> -    return -ENODEV;
>> -}
>> -
>>  /*
>>   * This function implements the MMIO based host to GuC interface.
>>   */
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h 
>> b/drivers/gpu/drm/i915/intel_uc.h
>> index 7703c9a..27e30aa 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -209,19 +209,8 @@ struct intel_huc {
>>  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
>>  int intel_guc_sample_forcewake(struct intel_guc *guc);
>> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len);
>>  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, 
>> u32 len);
>> -static inline int intel_guc_send(struct intel_guc *guc, const u32 
>> *action, u32 len)
>> -{
>> -    return guc->send(guc, action, len);
>> -}
>> -
>> -static inline void intel_guc_notify(struct intel_guc *guc)
>> -{
>> -    guc->notify(guc);
>> -}
>> -
>>  /* intel_guc_loader.c */
>>  int intel_guc_select_fw(struct intel_guc *guc);
>>  int intel_guc_init_hw(struct intel_guc *guc);



More information about the Intel-gfx mailing list