* [PATCH 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-02 12:39 [PATCH 1/3] drm/i915/guc: Move notification code into virtual function Michal Wajdeczko
@ 2017-05-02 12:39 ` Michal Wajdeczko
2017-05-02 16:54 ` Daniele Ceraolo Spurio
2017-05-02 12:39 ` [PATCH 3/3] HAX Enable GuC loading & submission Michal Wajdeczko
` (5 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Michal Wajdeczko @ 2017-05-02 12:39 UTC (permalink / raw)
To: intel-gfx
We are using some scratch registers in MMIO based send function.
Make their base and count flexible in preparation of upcoming
GuC firmware/hardware changes.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/intel_uc.c | 42 +++++++++++++++++++++++++++++++++--------
drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
2 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 72f49e6..73c3324 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -260,9 +260,35 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
__intel_uc_fw_fini(&dev_priv->huc.fw);
}
+static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
+{
+ GEM_BUG_ON(!guc->send_regs.base);
+ GEM_BUG_ON(!guc->send_regs.count);
+ GEM_BUG_ON(i >= guc->send_regs.count);
+
+ return _MMIO(guc->send_regs.base + 4 * i);
+}
+
+static void guc_init_send_regs(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ enum forcewake_domains fw_domains = 0;
+ u32 i;
+
+ guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
+ guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
+
+ for (i = 0; i < guc->send_regs.count; i++) {
+ fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
+ guc_send_reg(guc, i),
+ FW_REG_READ | FW_REG_WRITE);
+ }
+ guc->send_regs.fw_domains = fw_domains;
+}
+
static int guc_enable_communication(struct intel_guc *guc)
{
- /* XXX: placeholder for alternate setup */
+ guc_init_send_regs(guc);
guc->send = intel_guc_send_mmio;
return 0;
}
@@ -407,19 +433,19 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
int i;
int ret;
- if (WARN_ON(len < 1 || len > 15))
- return -EINVAL;
+ GEM_BUG_ON(!len);
+ GEM_BUG_ON(len > guc->send_regs.count);
mutex_lock(&guc->send_mutex);
- intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
+ intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
dev_priv->guc.action_count += 1;
dev_priv->guc.action_cmd = action[0];
for (i = 0; i < len; i++)
- I915_WRITE(SOFT_SCRATCH(i), action[i]);
+ I915_WRITE(guc_send_reg(guc, i), action[i]);
- POSTING_READ(SOFT_SCRATCH(i - 1));
+ POSTING_READ(guc_send_reg(guc, i - 1));
intel_guc_notify(guc);
@@ -428,7 +454,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
* Fast commands should still complete in 10us.
*/
ret = __intel_wait_for_register_fw(dev_priv,
- SOFT_SCRATCH(0),
+ guc_send_reg(guc, 0),
INTEL_GUC_RECV_MASK,
INTEL_GUC_RECV_MASK,
10, 10, &status);
@@ -450,7 +476,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
}
dev_priv->guc.action_status = status;
- intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
+ intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
mutex_unlock(&guc->send_mutex);
return ret;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 097289b..a37a8cc 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -205,6 +205,13 @@ struct intel_guc {
uint64_t submissions[I915_NUM_ENGINES];
uint32_t last_seqno[I915_NUM_ENGINES];
+ /* GuC's FW specific registers used in MMIO send */
+ struct {
+ u32 base;
+ u32 count;
+ u32 fw_domains; /* enum forcewake_domains */
+ } send_regs;
+
/* To serialize the intel_guc_send actions */
struct mutex send_mutex;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-02 12:39 ` [PATCH 2/3] drm/i915/guc: Make scratch register base and count flexible Michal Wajdeczko
@ 2017-05-02 16:54 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 21+ messages in thread
From: Daniele Ceraolo Spurio @ 2017-05-02 16:54 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On 02/05/17 05:39, Michal Wajdeczko wrote:
> We are using some scratch registers in MMIO based send function.
> Make their base and count flexible in preparation of upcoming
> GuC firmware/hardware changes.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_uc.c | 42 +++++++++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
> 2 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 72f49e6..73c3324 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -260,9 +260,35 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> __intel_uc_fw_fini(&dev_priv->huc.fw);
> }
>
> +static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
> +{
> + GEM_BUG_ON(!guc->send_regs.base);
> + GEM_BUG_ON(!guc->send_regs.count);
> + GEM_BUG_ON(i >= guc->send_regs.count);
> +
> + return _MMIO(guc->send_regs.base + 4 * i);
> +}
> +
> +static void guc_init_send_regs(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + enum forcewake_domains fw_domains = 0;
> + u32 i;
> +
> + guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
> + guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
> +
> + for (i = 0; i < guc->send_regs.count; i++) {
> + fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
> + guc_send_reg(guc, i),
> + FW_REG_READ | FW_REG_WRITE);
> + }
> + guc->send_regs.fw_domains = fw_domains;
> +}
> +
> static int guc_enable_communication(struct intel_guc *guc)
> {
> - /* XXX: placeholder for alternate setup */
Is it worth retaining this comment? We still expect the new _send
mechanism setup to be added here, right?
> + guc_init_send_regs(guc);
> guc->send = intel_guc_send_mmio;
> return 0;
> }
> @@ -407,19 +433,19 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> int i;
> int ret;
>
> - if (WARN_ON(len < 1 || len > 15))
> - return -EINVAL;
> + GEM_BUG_ON(!len);
> + GEM_BUG_ON(len > guc->send_regs.count);
Should we call out this change from WARN to GEM_BUG in the commit message?
Nitpicks aside,
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Regards,
Daniele
>
> mutex_lock(&guc->send_mutex);
> - intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
> + intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
>
> dev_priv->guc.action_count += 1;
> dev_priv->guc.action_cmd = action[0];
>
> for (i = 0; i < len; i++)
> - I915_WRITE(SOFT_SCRATCH(i), action[i]);
> + I915_WRITE(guc_send_reg(guc, i), action[i]);
>
> - POSTING_READ(SOFT_SCRATCH(i - 1));
> + POSTING_READ(guc_send_reg(guc, i - 1));
>
> intel_guc_notify(guc);
>
> @@ -428,7 +454,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> * Fast commands should still complete in 10us.
> */
> ret = __intel_wait_for_register_fw(dev_priv,
> - SOFT_SCRATCH(0),
> + guc_send_reg(guc, 0),
> INTEL_GUC_RECV_MASK,
> INTEL_GUC_RECV_MASK,
> 10, 10, &status);
> @@ -450,7 +476,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> }
> dev_priv->guc.action_status = status;
>
> - intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
> + intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
> mutex_unlock(&guc->send_mutex);
>
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 097289b..a37a8cc 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -205,6 +205,13 @@ struct intel_guc {
> uint64_t submissions[I915_NUM_ENGINES];
> uint32_t last_seqno[I915_NUM_ENGINES];
>
> + /* GuC's FW specific registers used in MMIO send */
> + struct {
> + u32 base;
> + u32 count;
> + u32 fw_domains; /* enum forcewake_domains */
> + } send_regs;
> +
> /* To serialize the intel_guc_send actions */
> struct mutex send_mutex;
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] HAX Enable GuC loading & submission
2017-05-02 12:39 [PATCH 1/3] drm/i915/guc: Move notification code into virtual function Michal Wajdeczko
2017-05-02 12:39 ` [PATCH 2/3] drm/i915/guc: Make scratch register base and count flexible Michal Wajdeczko
@ 2017-05-02 12:39 ` Michal Wajdeczko
2017-05-02 13:54 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/guc: Move notification code into virtual function Patchwork
` (4 subsequent siblings)
6 siblings, 0 replies; 21+ messages in thread
From: Michal Wajdeczko @ 2017-05-02 12:39 UTC (permalink / raw)
To: intel-gfx
This is just for CI testing, *** DO NOT MERGE ***
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/i915_params.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b6a7e36..abd2894 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
.verbose_state_checks = 1,
.nuclear_pageflip = 0,
.edp_vswing = 0,
- .enable_guc_loading = 0,
- .enable_guc_submission = 0,
+ .enable_guc_loading = 1,
+ .enable_guc_submission = 1,
.guc_log_level = -1,
.guc_firmware_path = NULL,
.huc_firmware_path = NULL,
@@ -221,12 +221,12 @@ MODULE_PARM_DESC(edp_vswing,
module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 0400);
MODULE_PARM_DESC(enable_guc_loading,
"Enable GuC firmware loading "
- "(-1=auto, 0=never [default], 1=if available, 2=required)");
+ "(-1=auto, 0=never, 1=if available [default], 2=required)");
module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, int, 0400);
MODULE_PARM_DESC(enable_guc_submission,
"Enable GuC submission "
- "(-1=auto, 0=never [default], 1=if available, 2=required)");
+ "(-1=auto, 0=never, 1=if available [default], 2=required)");
module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
MODULE_PARM_DESC(guc_log_level,
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/guc: Move notification code into virtual function
2017-05-02 12:39 [PATCH 1/3] drm/i915/guc: Move notification code into virtual function Michal Wajdeczko
2017-05-02 12:39 ` [PATCH 2/3] drm/i915/guc: Make scratch register base and count flexible Michal Wajdeczko
2017-05-02 12:39 ` [PATCH 3/3] HAX Enable GuC loading & submission Michal Wajdeczko
@ 2017-05-02 13:54 ` Patchwork
2017-05-02 16:37 ` [PATCH 1/3] " Daniele Ceraolo Spurio
` (3 subsequent siblings)
6 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2017-05-02 13:54 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/3] drm/i915/guc: Move notification code into virtual function
URL : https://patchwork.freedesktop.org/series/23805/
State : success
== Summary ==
Series 23805v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/23805/revisions/1/mbox/
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:430s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:427s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:579s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:491s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:530s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:496s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:479s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:409s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:408s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:420s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:477s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:473s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:451s
fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:550s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:440s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:562s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:453s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:485s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:415s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:532s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:407s
310077224306c08a82476bb616de679715e83485 drm-tip: 2017y-05m-02d-12h-04m-57s UTC integration manifest
3335977 HAX Enable GuC loading & submission
6a06ee0 drm/i915/guc: Make scratch register base and count flexible
0207b59 drm/i915/guc: Move notification code into virtual function
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4596/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/3] drm/i915/guc: Move notification code into virtual function
2017-05-02 12:39 [PATCH 1/3] drm/i915/guc: Move notification code into virtual function Michal Wajdeczko
` (2 preceding siblings ...)
2017-05-02 13:54 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/guc: Move notification code into virtual function Patchwork
@ 2017-05-02 16:37 ` Daniele Ceraolo Spurio
2017-05-02 21:33 ` Michal Wajdeczko
2017-05-04 12:48 ` [PATCH v2 2/3] drm/i915/guc: Make scratch register base and count flexible Michal Wajdeczko
` (2 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Daniele Ceraolo Spurio @ 2017-05-02 16:37 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On 02/05/17 05:39, Michal Wajdeczko wrote:
> Prepare for alternate GuC notification mechanism.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/intel_uc.c | 10 +++++++++-
> drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 7fd75ca..72f49e6 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -94,12 +94,20 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
> i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
> }
>
> +static void guc_write_irq_trigger(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;
>
> mutex_init(&guc->send_mutex);
> guc->send = intel_guc_send_nop;
> + guc->notify = guc_write_irq_trigger;
> }
>
> static void fetch_uc_fw(struct drm_i915_private *dev_priv,
> @@ -413,7 +421,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
>
> POSTING_READ(SOFT_SCRATCH(i - 1));
>
> - I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
> + intel_guc_notify(guc);
>
> /*
> * No GuC command should ever take longer than 10ms.
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 1e0eecd..097289b 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -210,6 +210,9 @@ struct intel_guc {
>
> /* GuC's FW specific send function */
> int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
> +
> + /* GuC's FW specific notify function */
> + void (*notify)(struct intel_guc *guc);
> };
>
> struct intel_huc {
> @@ -233,6 +236,10 @@ static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 l
> {
> return guc->send(guc, action, len);
> }
> +static inline void intel_guc_notify(struct intel_guc *guc)
> +{
> + guc->notify(guc);
> +}
>
personal preference: I would use guc->notify directly instead of adding
intel_guc_notify(). intel_guc_send() made more sense because a function
with that name already existed and by keeping it we minimized the
change, but I don't see such benefit with intel_guc_notify() and calling
the function pointer directly feels more in sync with what we do
elsewhere in the driver.
No strong feelings anyway, so:
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Regards,
Daniele
> /* intel_guc_loader.c */
> int intel_guc_select_fw(struct intel_guc *guc);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/3] drm/i915/guc: Move notification code into virtual function
2017-05-02 16:37 ` [PATCH 1/3] " Daniele Ceraolo Spurio
@ 2017-05-02 21:33 ` Michal Wajdeczko
0 siblings, 0 replies; 21+ messages in thread
From: Michal Wajdeczko @ 2017-05-02 21:33 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
On Tue, May 02, 2017 at 09:37:45AM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 02/05/17 05:39, Michal Wajdeczko wrote:
> > Prepare for alternate GuC notification mechanism.
> >
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_uc.c | 10 +++++++++-
> > drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
> > 2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> > index 7fd75ca..72f49e6 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > @@ -94,12 +94,20 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
> > i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
> > }
> >
> > +static void guc_write_irq_trigger(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;
> >
> > mutex_init(&guc->send_mutex);
> > guc->send = intel_guc_send_nop;
> > + guc->notify = guc_write_irq_trigger;
> > }
> >
> > static void fetch_uc_fw(struct drm_i915_private *dev_priv,
> > @@ -413,7 +421,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> >
> > POSTING_READ(SOFT_SCRATCH(i - 1));
> >
> > - I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
> > + intel_guc_notify(guc);
> >
> > /*
> > * No GuC command should ever take longer than 10ms.
> > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> > index 1e0eecd..097289b 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.h
> > +++ b/drivers/gpu/drm/i915/intel_uc.h
> > @@ -210,6 +210,9 @@ struct intel_guc {
> >
> > /* GuC's FW specific send function */
> > int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
> > +
> > + /* GuC's FW specific notify function */
> > + void (*notify)(struct intel_guc *guc);
> > };
> >
> > struct intel_huc {
> > @@ -233,6 +236,10 @@ static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 l
> > {
> > return guc->send(guc, action, len);
> > }
> > +static inline void intel_guc_notify(struct intel_guc *guc)
> > +{
> > + guc->notify(guc);
> > +}
> >
>
> personal preference: I would use guc->notify directly instead of adding
> intel_guc_notify(). intel_guc_send() made more sense because a function with
> that name already existed and by keeping it we minimized the change, but I
> don't see such benefit with intel_guc_notify() and calling the function
> pointer directly feels more in sync with what we do elsewhere in the driver.
>
Note that thanks to the compiler, resulting code is the same, but by keeping
intel_guc_notify() we are more flexible than when using hardcoded guc->notify.
Note that the same reason why we added intel_guc_send() "minimize the change"
can also be valid when in the future we may want to switch from vfun pointer
to other implementation that then can be hidden in that tiny inline that you
just wanted to kill.
-Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-02 12:39 [PATCH 1/3] drm/i915/guc: Move notification code into virtual function Michal Wajdeczko
` (3 preceding siblings ...)
2017-05-02 16:37 ` [PATCH 1/3] " Daniele Ceraolo Spurio
@ 2017-05-04 12:48 ` Michal Wajdeczko
2017-05-04 13:22 ` Jani Nikula
2017-05-05 11:35 ` [PATCH v3 " Michal Wajdeczko
2017-05-09 14:08 ` [PATCH v4 " Michal Wajdeczko
6 siblings, 1 reply; 21+ messages in thread
From: Michal Wajdeczko @ 2017-05-04 12:48 UTC (permalink / raw)
To: intel-gfx
We are using some scratch registers in MMIO based send function.
Make their base and count flexible in preparation of upcoming
GuC firmware/hardware changes. While around, change cmd len
parameter verification from WARN_ON to GEM_BUG_ON as we don't
need this all the time.
v2: call out WARN/GEM_BUG change in the commit msg (Daniele)
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/intel_uc.c | 41 ++++++++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 72f49e6..9d11c42 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -260,9 +260,36 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
__intel_uc_fw_fini(&dev_priv->huc.fw);
}
+static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
+{
+ GEM_BUG_ON(!guc->send_regs.base);
+ GEM_BUG_ON(!guc->send_regs.count);
+ GEM_BUG_ON(i >= guc->send_regs.count);
+
+ return _MMIO(guc->send_regs.base + 4 * i);
+}
+
+static void guc_init_send_regs(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ enum forcewake_domains fw_domains = 0;
+ u32 i;
+
+ guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
+ guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
+
+ for (i = 0; i < guc->send_regs.count; i++) {
+ fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
+ guc_send_reg(guc, i),
+ FW_REG_READ | FW_REG_WRITE);
+ }
+ guc->send_regs.fw_domains = fw_domains;
+}
+
static int guc_enable_communication(struct intel_guc *guc)
{
/* XXX: placeholder for alternate setup */
+ guc_init_send_regs(guc);
guc->send = intel_guc_send_mmio;
return 0;
}
@@ -407,19 +434,19 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
int i;
int ret;
- if (WARN_ON(len < 1 || len > 15))
- return -EINVAL;
+ GEM_BUG_ON(!len);
+ GEM_BUG_ON(len > guc->send_regs.count);
mutex_lock(&guc->send_mutex);
- intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
+ intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
dev_priv->guc.action_count += 1;
dev_priv->guc.action_cmd = action[0];
for (i = 0; i < len; i++)
- I915_WRITE(SOFT_SCRATCH(i), action[i]);
+ I915_WRITE(guc_send_reg(guc, i), action[i]);
- POSTING_READ(SOFT_SCRATCH(i - 1));
+ POSTING_READ(guc_send_reg(guc, i - 1));
intel_guc_notify(guc);
@@ -428,7 +455,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
* Fast commands should still complete in 10us.
*/
ret = __intel_wait_for_register_fw(dev_priv,
- SOFT_SCRATCH(0),
+ guc_send_reg(guc, 0),
INTEL_GUC_RECV_MASK,
INTEL_GUC_RECV_MASK,
10, 10, &status);
@@ -450,7 +477,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
}
dev_priv->guc.action_status = status;
- intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
+ intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
mutex_unlock(&guc->send_mutex);
return ret;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 097289b..a37a8cc 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -205,6 +205,13 @@ struct intel_guc {
uint64_t submissions[I915_NUM_ENGINES];
uint32_t last_seqno[I915_NUM_ENGINES];
+ /* GuC's FW specific registers used in MMIO send */
+ struct {
+ u32 base;
+ u32 count;
+ u32 fw_domains; /* enum forcewake_domains */
+ } send_regs;
+
/* To serialize the intel_guc_send actions */
struct mutex send_mutex;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-04 12:48 ` [PATCH v2 2/3] drm/i915/guc: Make scratch register base and count flexible Michal Wajdeczko
@ 2017-05-04 13:22 ` Jani Nikula
2017-05-04 16:26 ` Michal Wajdeczko
0 siblings, 1 reply; 21+ messages in thread
From: Jani Nikula @ 2017-05-04 13:22 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On Thu, 04 May 2017, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> We are using some scratch registers in MMIO based send function.
> Make their base and count flexible in preparation of upcoming
> GuC firmware/hardware changes. While around, change cmd len
> parameter verification from WARN_ON to GEM_BUG_ON as we don't
> need this all the time.
I'm not generally fond of caching the registers like this or adding
_MMIO() wrapping outside of i915_reg.h. Sure, we have some of that here
and there, but here it's hard to see the rationale because you do this
in preparation for something that we you're not sharing.
BR,
Jani.
>
> v2: call out WARN/GEM_BUG change in the commit msg (Daniele)
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/intel_uc.c | 41 ++++++++++++++++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
> 2 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 72f49e6..9d11c42 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -260,9 +260,36 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> __intel_uc_fw_fini(&dev_priv->huc.fw);
> }
>
> +static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
> +{
> + GEM_BUG_ON(!guc->send_regs.base);
> + GEM_BUG_ON(!guc->send_regs.count);
> + GEM_BUG_ON(i >= guc->send_regs.count);
> +
> + return _MMIO(guc->send_regs.base + 4 * i);
> +}
> +
> +static void guc_init_send_regs(struct intel_guc *guc)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + enum forcewake_domains fw_domains = 0;
> + u32 i;
> +
> + guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
> + guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
> +
> + for (i = 0; i < guc->send_regs.count; i++) {
> + fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
> + guc_send_reg(guc, i),
> + FW_REG_READ | FW_REG_WRITE);
> + }
> + guc->send_regs.fw_domains = fw_domains;
> +}
> +
> static int guc_enable_communication(struct intel_guc *guc)
> {
> /* XXX: placeholder for alternate setup */
> + guc_init_send_regs(guc);
> guc->send = intel_guc_send_mmio;
> return 0;
> }
> @@ -407,19 +434,19 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> int i;
> int ret;
>
> - if (WARN_ON(len < 1 || len > 15))
> - return -EINVAL;
> + GEM_BUG_ON(!len);
> + GEM_BUG_ON(len > guc->send_regs.count);
>
> mutex_lock(&guc->send_mutex);
> - intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
> + intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
>
> dev_priv->guc.action_count += 1;
> dev_priv->guc.action_cmd = action[0];
>
> for (i = 0; i < len; i++)
> - I915_WRITE(SOFT_SCRATCH(i), action[i]);
> + I915_WRITE(guc_send_reg(guc, i), action[i]);
>
> - POSTING_READ(SOFT_SCRATCH(i - 1));
> + POSTING_READ(guc_send_reg(guc, i - 1));
>
> intel_guc_notify(guc);
>
> @@ -428,7 +455,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> * Fast commands should still complete in 10us.
> */
> ret = __intel_wait_for_register_fw(dev_priv,
> - SOFT_SCRATCH(0),
> + guc_send_reg(guc, 0),
> INTEL_GUC_RECV_MASK,
> INTEL_GUC_RECV_MASK,
> 10, 10, &status);
> @@ -450,7 +477,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> }
> dev_priv->guc.action_status = status;
>
> - intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
> + intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
> mutex_unlock(&guc->send_mutex);
>
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 097289b..a37a8cc 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -205,6 +205,13 @@ struct intel_guc {
> uint64_t submissions[I915_NUM_ENGINES];
> uint32_t last_seqno[I915_NUM_ENGINES];
>
> + /* GuC's FW specific registers used in MMIO send */
> + struct {
> + u32 base;
> + u32 count;
> + u32 fw_domains; /* enum forcewake_domains */
> + } send_regs;
> +
> /* To serialize the intel_guc_send actions */
> struct mutex send_mutex;
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-04 13:22 ` Jani Nikula
@ 2017-05-04 16:26 ` Michal Wajdeczko
2017-05-04 20:52 ` Chris Wilson
0 siblings, 1 reply; 21+ messages in thread
From: Michal Wajdeczko @ 2017-05-04 16:26 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Thu, May 04, 2017 at 04:22:15PM +0300, Jani Nikula wrote:
> On Thu, 04 May 2017, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> > We are using some scratch registers in MMIO based send function.
> > Make their base and count flexible in preparation of upcoming
> > GuC firmware/hardware changes. While around, change cmd len
> > parameter verification from WARN_ON to GEM_BUG_ON as we don't
> > need this all the time.
>
> I'm not generally fond of caching the registers like this or adding
> _MMIO() wrapping outside of i915_reg.h. Sure, we have some of that here
> and there, but here it's hard to see the rationale because you do this
> in preparation for something that we you're not sharing.
>
I can't share details atm, but as commit message says, there will be a
change in both offsets and number of scratch registers.
Imho any wrapping around these values can't go to the i915_[guc_]reg.h file
as that file shall include only raw MMIO definitions, without any extra
logic that is based on GEN or PLATFORM or FW version.
Alternate approach would be, thanks to the already defined virtual function
send(), to create new send_mmio function(s) that will be 100% the same as
the old send_mmio except offset and count of the scratch registers.
Then we can benefit from most optimal implementation per GEN|PLATFORM|FW that
can run without reading cached regs offsets/count, but at the cost of extra
code that need to be maintained to be in sync with the original function.
And then someone else can point out that we missed code sharing opportunity.
I'm afraid there is no clear winner.
-Michal
> BR,
> Jani.
>
> >
> > v2: call out WARN/GEM_BUG change in the commit msg (Daniele)
> >
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_uc.c | 41 ++++++++++++++++++++++++++++++++++-------
> > drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
> > 2 files changed, 41 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> > index 72f49e6..9d11c42 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > @@ -260,9 +260,36 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> > __intel_uc_fw_fini(&dev_priv->huc.fw);
> > }
> >
> > +static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
> > +{
> > + GEM_BUG_ON(!guc->send_regs.base);
> > + GEM_BUG_ON(!guc->send_regs.count);
> > + GEM_BUG_ON(i >= guc->send_regs.count);
> > +
> > + return _MMIO(guc->send_regs.base + 4 * i);
> > +}
> > +
> > +static void guc_init_send_regs(struct intel_guc *guc)
> > +{
> > + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> > + enum forcewake_domains fw_domains = 0;
> > + u32 i;
> > +
> > + guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
> > + guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
> > +
> > + for (i = 0; i < guc->send_regs.count; i++) {
> > + fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
> > + guc_send_reg(guc, i),
> > + FW_REG_READ | FW_REG_WRITE);
> > + }
> > + guc->send_regs.fw_domains = fw_domains;
> > +}
> > +
> > static int guc_enable_communication(struct intel_guc *guc)
> > {
> > /* XXX: placeholder for alternate setup */
> > + guc_init_send_regs(guc);
> > guc->send = intel_guc_send_mmio;
> > return 0;
> > }
> > @@ -407,19 +434,19 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> > int i;
> > int ret;
> >
> > - if (WARN_ON(len < 1 || len > 15))
> > - return -EINVAL;
> > + GEM_BUG_ON(!len);
> > + GEM_BUG_ON(len > guc->send_regs.count);
> >
> > mutex_lock(&guc->send_mutex);
> > - intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
> > + intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
> >
> > dev_priv->guc.action_count += 1;
> > dev_priv->guc.action_cmd = action[0];
> >
> > for (i = 0; i < len; i++)
> > - I915_WRITE(SOFT_SCRATCH(i), action[i]);
> > + I915_WRITE(guc_send_reg(guc, i), action[i]);
> >
> > - POSTING_READ(SOFT_SCRATCH(i - 1));
> > + POSTING_READ(guc_send_reg(guc, i - 1));
> >
> > intel_guc_notify(guc);
> >
> > @@ -428,7 +455,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> > * Fast commands should still complete in 10us.
> > */
> > ret = __intel_wait_for_register_fw(dev_priv,
> > - SOFT_SCRATCH(0),
> > + guc_send_reg(guc, 0),
> > INTEL_GUC_RECV_MASK,
> > INTEL_GUC_RECV_MASK,
> > 10, 10, &status);
> > @@ -450,7 +477,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> > }
> > dev_priv->guc.action_status = status;
> >
> > - intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
> > + intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
> > mutex_unlock(&guc->send_mutex);
> >
> > return ret;
> > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> > index 097289b..a37a8cc 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.h
> > +++ b/drivers/gpu/drm/i915/intel_uc.h
> > @@ -205,6 +205,13 @@ struct intel_guc {
> > uint64_t submissions[I915_NUM_ENGINES];
> > uint32_t last_seqno[I915_NUM_ENGINES];
> >
> > + /* GuC's FW specific registers used in MMIO send */
> > + struct {
> > + u32 base;
> > + u32 count;
> > + u32 fw_domains; /* enum forcewake_domains */
> > + } send_regs;
> > +
> > /* To serialize the intel_guc_send actions */
> > struct mutex send_mutex;
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-04 16:26 ` Michal Wajdeczko
@ 2017-05-04 20:52 ` Chris Wilson
2017-05-05 6:08 ` Jani Nikula
0 siblings, 1 reply; 21+ messages in thread
From: Chris Wilson @ 2017-05-04 20:52 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
On Thu, May 04, 2017 at 06:26:04PM +0200, Michal Wajdeczko wrote:
> On Thu, May 04, 2017 at 04:22:15PM +0300, Jani Nikula wrote:
> > On Thu, 04 May 2017, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> > > We are using some scratch registers in MMIO based send function.
> > > Make their base and count flexible in preparation of upcoming
> > > GuC firmware/hardware changes. While around, change cmd len
> > > parameter verification from WARN_ON to GEM_BUG_ON as we don't
> > > need this all the time.
> >
> > I'm not generally fond of caching the registers like this or adding
> > _MMIO() wrapping outside of i915_reg.h. Sure, we have some of that here
> > and there, but here it's hard to see the rationale because you do this
> > in preparation for something that we you're not sharing.
> >
>
> I can't share details atm, but as commit message says, there will be a
> change in both offsets and number of scratch registers.
>
> Imho any wrapping around these values can't go to the i915_[guc_]reg.h file
> as that file shall include only raw MMIO definitions, without any extra
> logic that is based on GEN or PLATFORM or FW version.
The guc->send.base + offset approach is reasonable; it is certainly
the tried and trusted approach. I would stick with it, but we just can't
help with any suggestions without seeing the destination. Oh well, we
can dream that instead of using mmio space for datagrams they move to
ring (even WC will be better than a bunch of UC)!
Don't overqualify the ints though, u32 base is ok, but it could be unsigned
count (though an alternative would be u32 end, and even mark it as
GEM_DEBUG_DECL!) and definitely unsigned fw_domains as that is not
defined as being u32. (Just more than 32 domains is unlikely before
tomorrow ;)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-04 20:52 ` Chris Wilson
@ 2017-05-05 6:08 ` Jani Nikula
0 siblings, 0 replies; 21+ messages in thread
From: Jani Nikula @ 2017-05-05 6:08 UTC (permalink / raw)
To: Chris Wilson, Michal Wajdeczko; +Cc: intel-gfx
On Thu, 04 May 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, May 04, 2017 at 06:26:04PM +0200, Michal Wajdeczko wrote:
>> On Thu, May 04, 2017 at 04:22:15PM +0300, Jani Nikula wrote:
>> > On Thu, 04 May 2017, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
>> > > We are using some scratch registers in MMIO based send function.
>> > > Make their base and count flexible in preparation of upcoming
>> > > GuC firmware/hardware changes. While around, change cmd len
>> > > parameter verification from WARN_ON to GEM_BUG_ON as we don't
>> > > need this all the time.
>> >
>> > I'm not generally fond of caching the registers like this or adding
>> > _MMIO() wrapping outside of i915_reg.h. Sure, we have some of that here
>> > and there, but here it's hard to see the rationale because you do this
>> > in preparation for something that we you're not sharing.
>> >
>>
>> I can't share details atm, but as commit message says, there will be a
>> change in both offsets and number of scratch registers.
>>
>> Imho any wrapping around these values can't go to the i915_[guc_]reg.h file
>> as that file shall include only raw MMIO definitions, without any extra
>> logic that is based on GEN or PLATFORM or FW version.
>
> The guc->send.base + offset approach is reasonable; it is certainly
> the tried and trusted approach. I would stick with it, but we just can't
> help with any suggestions without seeing the destination.
That pretty much sums it up.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-02 12:39 [PATCH 1/3] drm/i915/guc: Move notification code into virtual function Michal Wajdeczko
` (4 preceding siblings ...)
2017-05-04 12:48 ` [PATCH v2 2/3] drm/i915/guc: Make scratch register base and count flexible Michal Wajdeczko
@ 2017-05-05 11:35 ` Michal Wajdeczko
2017-05-08 11:31 ` Joonas Lahtinen
2017-05-09 14:08 ` [PATCH v4 " Michal Wajdeczko
6 siblings, 1 reply; 21+ messages in thread
From: Michal Wajdeczko @ 2017-05-05 11:35 UTC (permalink / raw)
To: intel-gfx
We are using some scratch registers in MMIO based send function.
Make their base and count flexible in preparation of upcoming
GuC firmware/hardware changes. While around, change cmd len
parameter verification from WARN_ON to GEM_BUG_ON as we don't
need this all the time.
v2: call out WARN/GEM_BUG change in the commit msg (Daniele)
v3: don't overqualify the ints (Chris)
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/intel_uc.c | 41 ++++++++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 72f49e6..9d11c42 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -260,9 +260,36 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
__intel_uc_fw_fini(&dev_priv->huc.fw);
}
+static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
+{
+ GEM_BUG_ON(!guc->send_regs.base);
+ GEM_BUG_ON(!guc->send_regs.count);
+ GEM_BUG_ON(i >= guc->send_regs.count);
+
+ return _MMIO(guc->send_regs.base + 4 * i);
+}
+
+static void guc_init_send_regs(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ enum forcewake_domains fw_domains = 0;
+ u32 i;
+
+ guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
+ guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
+
+ for (i = 0; i < guc->send_regs.count; i++) {
+ fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
+ guc_send_reg(guc, i),
+ FW_REG_READ | FW_REG_WRITE);
+ }
+ guc->send_regs.fw_domains = fw_domains;
+}
+
static int guc_enable_communication(struct intel_guc *guc)
{
/* XXX: placeholder for alternate setup */
+ guc_init_send_regs(guc);
guc->send = intel_guc_send_mmio;
return 0;
}
@@ -407,19 +434,19 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
int i;
int ret;
- if (WARN_ON(len < 1 || len > 15))
- return -EINVAL;
+ GEM_BUG_ON(!len);
+ GEM_BUG_ON(len > guc->send_regs.count);
mutex_lock(&guc->send_mutex);
- intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
+ intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
dev_priv->guc.action_count += 1;
dev_priv->guc.action_cmd = action[0];
for (i = 0; i < len; i++)
- I915_WRITE(SOFT_SCRATCH(i), action[i]);
+ I915_WRITE(guc_send_reg(guc, i), action[i]);
- POSTING_READ(SOFT_SCRATCH(i - 1));
+ POSTING_READ(guc_send_reg(guc, i - 1));
intel_guc_notify(guc);
@@ -428,7 +455,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
* Fast commands should still complete in 10us.
*/
ret = __intel_wait_for_register_fw(dev_priv,
- SOFT_SCRATCH(0),
+ guc_send_reg(guc, 0),
INTEL_GUC_RECV_MASK,
INTEL_GUC_RECV_MASK,
10, 10, &status);
@@ -450,7 +477,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
}
dev_priv->guc.action_status = status;
- intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
+ intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
mutex_unlock(&guc->send_mutex);
return ret;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 097289b..4dd39a7 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -205,6 +205,13 @@ struct intel_guc {
uint64_t submissions[I915_NUM_ENGINES];
uint32_t last_seqno[I915_NUM_ENGINES];
+ /* GuC's FW specific registers used in MMIO send */
+ struct {
+ u32 base;
+ unsigned int count;
+ unsigned int fw_domains; /* enum forcewake_domains */
+ } send_regs;
+
/* To serialize the intel_guc_send actions */
struct mutex send_mutex;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v3 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-05 11:35 ` [PATCH v3 " Michal Wajdeczko
@ 2017-05-08 11:31 ` Joonas Lahtinen
2017-05-08 12:07 ` Jani Nikula
0 siblings, 1 reply; 21+ messages in thread
From: Joonas Lahtinen @ 2017-05-08 11:31 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On pe, 2017-05-05 at 11:35 +0000, Michal Wajdeczko wrote:
> We are using some scratch registers in MMIO based send function.
> Make their base and count flexible in preparation of upcoming
> GuC firmware/hardware changes. While around, change cmd len
> parameter verification from WARN_ON to GEM_BUG_ON as we don't
> need this all the time.
>
> v2: call out WARN/GEM_BUG change in the commit msg (Daniele)
> v3: don't overqualify the ints (Chris)
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
<SNIP>
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -205,6 +205,13 @@ struct intel_guc {
> uint64_t submissions[I915_NUM_ENGINES];
> uint32_t last_seqno[I915_NUM_ENGINES];
>
> + /* GuC's FW specific registers used in MMIO send */
> + struct {
> + u32 base;
> + unsigned int count;
> + unsigned int fw_domains; /* enum forcewake_domains */
As discussed in IRC, split intel_uncore.h with what is reasonable
without refactoring, and untangle the order to make the type what it is
inside the function.
Regards, Joonas
PS. I personally don't like the enum typed bitfields, but that's an
another discussion (that's been had in the past).
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v3 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-08 11:31 ` Joonas Lahtinen
@ 2017-05-08 12:07 ` Jani Nikula
0 siblings, 0 replies; 21+ messages in thread
From: Jani Nikula @ 2017-05-08 12:07 UTC (permalink / raw)
To: Joonas Lahtinen, Michal Wajdeczko, intel-gfx
On Mon, 08 May 2017, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:
> PS. I personally don't like the enum typed bitfields, but that's an
> another discussion (that's been had in the past).
I'm with you on this one. It's semi-okay to define the bits as enums,
but IMO a variable of an enum type should only ever be used to hold the
enumerated values. Too bad C doesn't give us strong type checking for
enums.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-02 12:39 [PATCH 1/3] drm/i915/guc: Move notification code into virtual function Michal Wajdeczko
` (5 preceding siblings ...)
2017-05-05 11:35 ` [PATCH v3 " Michal Wajdeczko
@ 2017-05-09 14:08 ` Michal Wajdeczko
2017-05-10 10:24 ` Joonas Lahtinen
6 siblings, 1 reply; 21+ messages in thread
From: Michal Wajdeczko @ 2017-05-09 14:08 UTC (permalink / raw)
To: intel-gfx
We are using some scratch registers in MMIO based send function.
Make their base and count flexible in preparation of upcoming
GuC firmware/hardware changes. While around, change cmd len
parameter verification from WARN_ON to GEM_BUG_ON as we don't
need this all the time.
v2: call out WARN/GEM_BUG change in the commit msg (Daniele)
v3: don't overqualify the ints (Chris)
v4: rebase and use proper enum
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/intel_uc.c | 41 ++++++++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_uc.h | 7 +++++++
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 72f49e6..07c5658 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -260,9 +260,36 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
__intel_uc_fw_fini(&dev_priv->huc.fw);
}
+static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
+{
+ GEM_BUG_ON(!guc->send_regs.base);
+ GEM_BUG_ON(!guc->send_regs.count);
+ GEM_BUG_ON(i >= guc->send_regs.count);
+
+ return _MMIO(guc->send_regs.base + 4 * i);
+}
+
+static void guc_init_send_regs(struct intel_guc *guc)
+{
+ struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ enum forcewake_domains fw_domains = 0;
+ unsigned int i;
+
+ guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
+ guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
+
+ for (i = 0; i < guc->send_regs.count; i++) {
+ fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
+ guc_send_reg(guc, i),
+ FW_REG_READ | FW_REG_WRITE);
+ }
+ guc->send_regs.fw_domains = fw_domains;
+}
+
static int guc_enable_communication(struct intel_guc *guc)
{
/* XXX: placeholder for alternate setup */
+ guc_init_send_regs(guc);
guc->send = intel_guc_send_mmio;
return 0;
}
@@ -407,19 +434,19 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
int i;
int ret;
- if (WARN_ON(len < 1 || len > 15))
- return -EINVAL;
+ GEM_BUG_ON(!len);
+ GEM_BUG_ON(len > guc->send_regs.count);
mutex_lock(&guc->send_mutex);
- intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
+ intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
dev_priv->guc.action_count += 1;
dev_priv->guc.action_cmd = action[0];
for (i = 0; i < len; i++)
- I915_WRITE(SOFT_SCRATCH(i), action[i]);
+ I915_WRITE(guc_send_reg(guc, i), action[i]);
- POSTING_READ(SOFT_SCRATCH(i - 1));
+ POSTING_READ(guc_send_reg(guc, i - 1));
intel_guc_notify(guc);
@@ -428,7 +455,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
* Fast commands should still complete in 10us.
*/
ret = __intel_wait_for_register_fw(dev_priv,
- SOFT_SCRATCH(0),
+ guc_send_reg(guc, 0),
INTEL_GUC_RECV_MASK,
INTEL_GUC_RECV_MASK,
10, 10, &status);
@@ -450,7 +477,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
}
dev_priv->guc.action_status = status;
- intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
+ intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
mutex_unlock(&guc->send_mutex);
return ret;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 097289b..53a3388 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -205,6 +205,13 @@ struct intel_guc {
uint64_t submissions[I915_NUM_ENGINES];
uint32_t last_seqno[I915_NUM_ENGINES];
+ /* GuC's FW specific registers used in MMIO send */
+ struct {
+ u32 base;
+ unsigned int count;
+ enum forcewake_domains fw_domains;
+ } send_regs;
+
/* To serialize the intel_guc_send actions */
struct mutex send_mutex;
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v4 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-09 14:08 ` [PATCH v4 " Michal Wajdeczko
@ 2017-05-10 10:24 ` Joonas Lahtinen
2017-05-10 10:32 ` Jani Nikula
0 siblings, 1 reply; 21+ messages in thread
From: Joonas Lahtinen @ 2017-05-10 10:24 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On ti, 2017-05-09 at 14:08 +0000, Michal Wajdeczko wrote:
> We are using some scratch registers in MMIO based send function.
> Make their base and count flexible in preparation of upcoming
> GuC firmware/hardware changes. While around, change cmd len
> parameter verification from WARN_ON to GEM_BUG_ON as we don't
> need this all the time.
>
> v2: call out WARN/GEM_BUG change in the commit msg (Daniele)
> v3: don't overqualify the ints (Chris)
> v4: rebase and use proper enum
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
I suppose this could've had "# v2" at the end, unless Daniele acked all
the remaining changes.
Anyway, looks good to me with the changes.
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Could use an A-b from Jani for disagree and commit stamp as he
initially objected the way of implementing this.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-10 10:24 ` Joonas Lahtinen
@ 2017-05-10 10:32 ` Jani Nikula
2017-05-10 11:45 ` Joonas Lahtinen
0 siblings, 1 reply; 21+ messages in thread
From: Jani Nikula @ 2017-05-10 10:32 UTC (permalink / raw)
To: Joonas Lahtinen, Michal Wajdeczko, intel-gfx
On Wed, 10 May 2017, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:
> On ti, 2017-05-09 at 14:08 +0000, Michal Wajdeczko wrote:
>> We are using some scratch registers in MMIO based send function.
>> Make their base and count flexible in preparation of upcoming
>> GuC firmware/hardware changes. While around, change cmd len
>> parameter verification from WARN_ON to GEM_BUG_ON as we don't
>> need this all the time.
>>
>> v2: call out WARN/GEM_BUG change in the commit msg (Daniele)
>> v3: don't overqualify the ints (Chris)
>> v4: rebase and use proper enum
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> I suppose this could've had "# v2" at the end, unless Daniele acked all
> the remaining changes.
>
> Anyway, looks good to me with the changes.
>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> Could use an A-b from Jani for disagree and commit stamp as he
> initially objected the way of implementing this.
Acked-by: Jani Nikula <jani.nikula@intel.com>
>
> Regards, Joonas
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 2/3] drm/i915/guc: Make scratch register base and count flexible
2017-05-10 10:32 ` Jani Nikula
@ 2017-05-10 11:45 ` Joonas Lahtinen
0 siblings, 0 replies; 21+ messages in thread
From: Joonas Lahtinen @ 2017-05-10 11:45 UTC (permalink / raw)
To: Jani Nikula, Michal Wajdeczko, intel-gfx
On ke, 2017-05-10 at 13:32 +0300, Jani Nikula wrote:
> > On Wed, 10 May 2017, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:
> >
> > On ti, 2017-05-09 at 14:08 +0000, Michal Wajdeczko wrote:
> > >
> > > We are using some scratch registers in MMIO based send function.
> > > Make their base and count flexible in preparation of upcoming
> > > GuC firmware/hardware changes. While around, change cmd len
> > > parameter verification from WARN_ON to GEM_BUG_ON as we don't
> > > need this all the time.
> > >
> > > v2: call out WARN/GEM_BUG change in the commit msg (Daniele)
> > > v3: don't overqualify the ints (Chris)
> > > v4: rebase and use proper enum
> > >
> > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > > Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >
> > I suppose this could've had "# v2" at the end, unless Daniele acked all
> > the remaining changes.
> >
> > Anyway, looks good to me with the changes.
> >
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >
> > Could use an A-b from Jani for disagree and commit stamp as he
> > initially objected the way of implementing this.
>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
Michal, could you apply the tags and re-send the series so CI picks it
up.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread