On Thu, Nov 24, 2016 at 05:13:21PM +0100, Arkadiusz Hiler wrote:
> guc_send(), guc_recv() and related functions were introduced in the
> i915_guc_submission.c and their scope was limited only to that file.
>
> Those are not submission specific though.
> This patch moves moves them to intel_uc.c with intel_ prefix added.
>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Michal Winiarski <michal.winiarski@xxxxxxxxx>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/i915_guc_submission.c | 131 +++------------------------
> drivers/gpu/drm/i915/intel_uc.c | 138 +++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_uc.h | 9 ++
> 4 files changed, 159 insertions(+), 122 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_uc.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 580602d..3c30916 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -55,7 +55,8 @@ i915-y += i915_cmd_parser.o \
> intel_uncore.o
>
> # general-purpose microcontroller (GuC) support
> -i915-y += intel_guc_loader.o \
> +i915-y += intel_uc.o \
> + intel_guc_loader.o \
> i915_guc_submission.o
>
> # autogenerated null render state
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index f8957df..d1f0d6d 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -49,7 +49,7 @@
> * Firmware writes a success/fail code back to the action register after
> * processes the request. The kernel driver polls waiting for this update and
> * then proceeds.
> - * See guc_send()
> + * See intel_guc_send()
> *
> * Doorbells:
> * Doorbells are interrupts to uKernel. A doorbell is a single cache line (QW)
> @@ -66,71 +66,6 @@
> */
>
> /*
> - * Read GuC command/status register (SOFT_SCRATCH_0)
> - * Return true if it contains a response rather than a command
> - */
> -static inline bool guc_recv(struct drm_i915_private *dev_priv, u32 *status)
> -{
> - u32 val = I915_READ(SOFT_SCRATCH(0));
> - *status = val;
> - return INTEL_GUC_RECV_IS_RESPONSE(val);
> -}
> -
> -static int guc_send(struct intel_guc *guc, u32 *data, u32 len)
> -{
> - struct drm_i915_private *dev_priv = guc_to_i915(guc);
> - u32 status;
> - int i;
> - int ret;
> -
> - if (WARN_ON(len < 1 || len > 15))
> - return -EINVAL;
> -
> - mutex_lock(&guc->send_mutex);
> - intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> -
> - dev_priv->guc.action_count += 1;
> - dev_priv->guc.action_cmd = data[0];
> -
> - for (i = 0; i < len; i++)
> - I915_WRITE(SOFT_SCRATCH(i), data[i]);
> -
> - POSTING_READ(SOFT_SCRATCH(i - 1));
> -
> - I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
> -
> - /*
> - * Fast commands should complete in less than 10us, so sample quickly
> - * up to that length of time, then switch to a slower sleep-wait loop.
> - * No INTEL_GUC_ACTION command should ever take longer than 10ms.
> - */
> - ret = wait_for_us(guc_recv(dev_priv, &status), 10);
> - if (ret)
> - ret = wait_for(guc_recv(dev_priv, &status), 10);
> - if (status != INTEL_GUC_RECV_STATUS_SUCCESS) {
> - /*
> - * Either the GuC explicitly returned an error (which
> - * we convert to -EIO here) or no response at all was
> - * received within the timeout limit (-ETIMEDOUT)
> - */
> - if (ret != -ETIMEDOUT)
> - ret = -EIO;
> -
> - DRM_WARN("Action 0x%X failed; ret=%d status=0x%08X response=0x%08X\n",
> - data[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
> -
> - dev_priv->guc.action_fail += 1;
> - dev_priv->guc.action_err = ret;
> - }
> - dev_priv->guc.action_status = status;
> -
> - intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> - mutex_unlock(&guc->send_mutex);
> -
> - return ret;
> -}
> -
> -/*
> * Tell the GuC to allocate or deallocate a specific doorbell
> */
>
> @@ -142,7 +77,7 @@ static int guc_allocate_doorbell(struct intel_guc *guc,
> data[0] = INTEL_GUC_ACTION_ALLOCATE_DOORBELL;
> data[1] = client->ctx_index;
>
> - return guc_send(guc, data, 2);
> + return intel_guc_send(guc, data, 2);
> }
>
> static int guc_release_doorbell(struct intel_guc *guc,
> @@ -153,53 +88,7 @@ static int guc_release_doorbell(struct intel_guc *guc,
> data[0] = INTEL_GUC_ACTION_DEALLOCATE_DOORBELL;
> data[1] = client->ctx_index;
>
> - return guc_send(guc, data, 2);
> -}
> -
> -static int guc_sample_forcewake(struct intel_guc *guc,
> - struct i915_guc_client *client)
> -{
> - struct drm_i915_private *dev_priv = guc_to_i915(guc);
> - u32 data[2];
> -
> - data[0] = INTEL_GUC_ACTION_SAMPLE_FORCEWAKE;
> - /* WaRsDisableCoarsePowerGating:skl,bxt */
> - if (!intel_enable_rc6() || NEEDS_WaRsDisableCoarsePowerGating(dev_priv))
> - data[1] = 0;
> - else
> - /* bit 0 and 1 are for Render and Media domain separately */
> - data[1] = GUC_FORCEWAKE_RENDER | GUC_FORCEWAKE_MEDIA;
> -
> - return guc_send(guc, data, ARRAY_SIZE(data));
> -}
> -
> -static int guc_logbuffer_flush_complete(struct intel_guc *guc)
> -{
> - u32 data[1];
> -
> - data[0] = INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE;
> -
> - return guc_send(guc, data, 1);
> -}
> -
> -static int guc_force_logbuffer_flush(struct intel_guc *guc)
> -{
> - u32 data[2];
> -
> - data[0] = INTEL_GUC_ACTION_FORCE_LOG_BUFFER_FLUSH;
> - data[1] = 0;
> -
> - return guc_send(guc, data, 2);
> -}
> -
> -static int guc_logging_control(struct intel_guc *guc, u32 control_val)
> -{
> - u32 data[2];
> -
> - data[0] = INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING;
> - data[1] = control_val;
> -
> - return guc_send(guc, data, 2);
> + return intel_guc_send(guc, data, 2);
> }
>
> /*
> @@ -297,7 +186,7 @@ select_doorbell_register(struct intel_guc *guc, uint32_t priority)
> * Select, assign and relase doorbell cachelines
> *
> * These functions track which doorbell cachelines are in use.
> - * The data they manipulate is protected by the guc_send lock.
> + * The data they manipulate is protected by the intel_guc_send lock.
> */
>
> static uint32_t select_doorbell_cacheline(struct intel_guc *guc)
> @@ -1525,7 +1414,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
> }
>
> guc->execbuf_client = client;
> - guc_sample_forcewake(guc, client);
> + intel_guc_sample_forcewake(guc, client);
> guc_init_doorbell_hw(guc);
>
> /* Take over from manual control of ELSP (execlists) */
> @@ -1595,7 +1484,7 @@ int intel_guc_suspend(struct drm_device *dev)
> /* first page is shared data with GuC */
> data[2] = i915_ggtt_offset(ctx->engine[RCS].state);
>
> - return guc_send(guc, data, ARRAY_SIZE(data));
> + return intel_guc_send(guc, data, ARRAY_SIZE(data));
> }
>
>
> @@ -1623,7 +1512,7 @@ int intel_guc_resume(struct drm_device *dev)
> /* first page is shared data with GuC */
> data[2] = i915_ggtt_offset(ctx->engine[RCS].state);
>
> - return guc_send(guc, data, ARRAY_SIZE(data));
> + return intel_guc_send(guc, data, ARRAY_SIZE(data));
> }
>
> void i915_guc_capture_logs(struct drm_i915_private *dev_priv)
> @@ -1634,7 +1523,7 @@ void i915_guc_capture_logs(struct drm_i915_private *dev_priv)
> * time, so get/put should be really quick.
> */
> intel_runtime_pm_get(dev_priv);
> - guc_logbuffer_flush_complete(&dev_priv->guc);
> + intel_guc_logbuffer_flush_complete(&dev_priv->guc);
> intel_runtime_pm_put(dev_priv);
> }
>
> @@ -1652,7 +1541,7 @@ void i915_guc_flush_logs(struct drm_i915_private *dev_priv)
> flush_work(&dev_priv->guc.log.flush_work);
>
> /* Ask GuC to update the log buffer state */
> - guc_force_logbuffer_flush(&dev_priv->guc);
> + intel_guc_force_logbuffer_flush(&dev_priv->guc);
>
> /* GuC would have updated log buffer by now, so capture it */
> i915_guc_capture_logs(dev_priv);
> @@ -1693,7 +1582,7 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
> if (!log_param.logging_enabled && (i915.guc_log_level < 0))
> return 0;
>
> - ret = guc_logging_control(&dev_priv->guc, log_param.value);
> + ret = intel_guc_logging_control(&dev_priv->guc, log_param.value);
> if (ret < 0) {
> DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", ret);
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> new file mode 100644
> index 0000000..497c9c3
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -0,0 +1,138 @@
> +/*
> + * Copyright © 2016 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"
> +#include "intel_uc.h"
> +
> +/*
> + * Read GuC command/status register (SOFT_SCRATCH_0)
> + * Return true if it contains a response rather than a command
> + */
> +bool intel_guc_recv(struct drm_i915_private *dev_priv, u32 *status)
> +{
> + u32 val = I915_READ(SOFT_SCRATCH(0));
> + *status = val;
> + return INTEL_GUC_RECV_IS_RESPONSE(val);
> +}
> +
> +int intel_guc_send(struct intel_guc *guc, u32 *data, u32 len)
Should be const u32 *data.
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + u32 status;
> + int i;
> + int ret;
> +
> + if (WARN_ON(len < 1 || len > 15))
> + return -EINVAL;
> +
> + mutex_lock(&guc->send_mutex);
> + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> +
> + dev_priv->guc.action_count += 1;
> + dev_priv->guc.action_cmd = data[0];
> +
> + for (i = 0; i < len; i++)
> + I915_WRITE(SOFT_SCRATCH(i), data[i]);
> +
> + POSTING_READ(SOFT_SCRATCH(i - 1));
> +
> + I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
> +
> + /*
> + * Fast commands should complete in less than 10us, so sample quickly
> + * up to that length of time, then switch to a slower sleep-wait loop.
> + * No inte_guc_send command should ever take longer than 10ms.
> + */
> + ret = wait_for_us(intel_guc_recv(dev_priv, &status), 10);
> + if (ret)
> + ret = wait_for(intel_guc_recv(dev_priv, &status), 10);
> + if (status != INTEL_GUC_RECV_STATUS_SUCCESS) {
> + /*
> + * Either the GuC explicitly returned an error (which
> + * we convert to -EIO here) or no response at all was
> + * received within the timeout limit (-ETIMEDOUT)
> + */
> + if (ret != -ETIMEDOUT)
> + ret = -EIO;
> +
> + DRM_WARN("Action 0x%X failed; ret=%d status=0x%08X response=0x%08X\n",
> + data[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
DRM_WARN excludes the function name, it lacks context, so the message
needs to be cystal clear and identifiable (remember it is also user
facing).
> +
> + dev_priv->guc.action_fail += 1;
> + dev_priv->guc.action_err = ret;
> + }
> + dev_priv->guc.action_status = status;
> +
> + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> + mutex_unlock(&guc->send_mutex);
> +
> + return ret;
> +}
> +
> +int intel_guc_sample_forcewake(struct intel_guc *guc,
> + struct i915_guc_client *client)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + u32 data[2];
> +
> + data[0] = INTEL_GUC_ACTION_SAMPLE_FORCEWAKE;
> + /* WaRsDisableCoarsePowerGating:skl,bxt */
> + if (!intel_enable_rc6() || NEEDS_WaRsDisableCoarsePowerGating(dev_priv))
> + data[1] = 0;
> + else
> + /* bit 0 and 1 are for Render and Media domain separately */
> + data[1] = GUC_FORCEWAKE_RENDER | GUC_FORCEWAKE_MEDIA;
> +
> + return intel_guc_send(guc, data, ARRAY_SIZE(data));
> +}
> +
> +int intel_guc_logbuffer_flush_complete(struct intel_guc *guc)
> +{
> + u32 data[1];
> +
> + data[0] = INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE;
> +
> + return intel_guc_send(guc, data, 1);
> +}
> +
> +int intel_guc_force_logbuffer_flush(struct intel_guc *guc)
> +{
> + u32 data[2];
> +
> + data[0] = INTEL_GUC_ACTION_FORCE_LOG_BUFFER_FLUSH;
> + data[1] = 0;
> +
> + return intel_guc_send(guc, data, 2);
> +}
> +
> +int intel_guc_logging_control(struct intel_guc *guc, u32 control_val)
> +{
> + u32 data[2];
> +
> + data[0] = INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING;
> + data[1] = control_val;
> +
> + return intel_guc_send(guc, data, 2);
u32 pkt[] = {
INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING,
control_val,
};
return intel_guc_send(guc, pkt, ARRAY_SIZE(pkt);
Or being fancy
return intel_guc_send_cmd(guc, {
INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING,
control_val
});
#define intel_guc_send_cmd(G, PKT) ({ \
u32 pkt__[] = PKT;\
intel_guc_send(G, pkt__, ARRAY_SIZE(pkt__)); \
)}
or intel_guc_send_pkt
> +}
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 4b4a91e..0a55e5a 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -168,6 +168,15 @@ struct intel_guc {
> struct mutex send_mutex;
> };
>
> +/* intel_uc.c */
> +bool intel_guc_recv(struct drm_i915_private *dev_priv, u32 *status);
> +int intel_guc_send(struct intel_guc *guc, u32 *data, u32 len);
> +int intel_guc_sample_forcewake(struct intel_guc *guc,
> + struct i915_guc_client *client);
> +int intel_guc_logbuffer_flush_complete(struct intel_guc *guc);
> +int intel_guc_force_logbuffer_flush(struct intel_guc *guc);
> +int intel_guc_logging_control(struct intel_guc *guc, u32 control_val);
Ugh. (Names chosen based on enum, but lack consistency with our code)
intel_guc_log_control();
intel_guc_log_flush()
intel_guc_log_flush_complete()
Looks ok, but the code could do with some polish. The benefit of
exposing it ;)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
