Re: [Intel-gfx] [PATCH 3/9] drm/i915/pcode: Extend pcode functions for multiple gt's
On Fri, 29 Apr 2022 05:58:21 -0700, Rodrigo Vivi wrote: > > > @@ -1251,7 +1251,7 @@ static int i915_drm_resume(struct drm_device *dev) > > > > disable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > > > - ret = intel_pcode_init(dev_priv); > > + ret = intel_gt_pcode_init(dev_priv); > > I didn't like we have this indirection i915 -> gt -> i915... > At the same time I understand you don't want to duplicate the for_each with > the error msg and all in here. > > So, what about having in this file a > static int __init_pcode(dev_priv) > ?! Sure, will fix. > > diff --git a/drivers/gpu/drm/i915/intel_pcode.c > > b/drivers/gpu/drm/i915/intel_pcode.c > > index ac727546868e..66020b2e461f 100644 > > --- a/drivers/gpu/drm/i915/intel_pcode.c > > +++ b/drivers/gpu/drm/i915/intel_pcode.c > > @@ -52,14 +52,12 @@ static int gen7_check_mailbox_status(u32 mbox) > > } > > } > > > > -static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox, > > +static int intel_pcode_rw(struct intel_uncore *uncore, u32 mbox, > > I'm not sure if I like the idea of the renaming here... > I mean, it looks nicer indeed, but at the same time the "intel_" > make it looks it is exported one. Sure, will fix. > > --- a/drivers/gpu/drm/i915/intel_pcode.h > > +++ b/drivers/gpu/drm/i915/intel_pcode.h > > @@ -8,17 +8,32 @@ > > > > #include > > > > +struct intel_uncore; > > struct drm_i915_private; > > > > -int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32 > > *val1); > > -int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 > > val, > > - int fast_timeout_us, int slow_timeout_ms); > > -#define snb_pcode_write(i915, mbox, val) \ > > +int intel_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32 > > *val1); > > + > > +int intel_pcode_write_timeout(struct intel_uncore *uncore, u32 mbox, u32 > > val, > > + int fast_timeout_us, int slow_timeout_ms); > > + > > +#define intel_pcode_write(uncore, mbox, val) \ > > + intel_pcode_write_timeout(uncore, mbox, val, 500, 0) > > + > > +int intel_pcode_request(struct intel_uncore *uncore, u32 mbox, u32 request, > > + u32 reply_mask, u32 reply, int timeout_base_ms); > > + > > +#define snb_pcode_read(i915, mbox, val, val1) \ > > + intel_pcode_read(&(i915)->uncore, mbox, val, val1) > > + > > +#define snb_pcode_write_timeout(i915, mbox, val, fast_timeout_us, > > slow_timeout_ms) \ > > + intel_pcode_write_timeout(&(i915)->uncore, mbox, val, fast_timeout_us, > > slow_timeout_ms) > > + > > +#define snb_pcode_write(i915, mbox, val) \ > > snb_pcode_write_timeout(i915, mbox, val, 500, 0) > > > > -int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request, > > - u32 reply_mask, u32 reply, int timeout_base_ms); > > +#define skl_pcode_request(i915, mbox, request, reply_mask, reply, > > timeout_base_ms) \ > > + intel_pcode_request(&(i915)->uncore, mbox, request, reply_mask, reply, > > timeout_base_ms) > > and for the exported one, since we are renaming it, shouldn't we rename > all the users instead of creating these defines? Ok, in that case we might as well retain the original function names (snb_/skl_ etc. and just change the first argument to uncore)? So will do that in the next rev unless we think we want to rename everything to intel_? Thanks. -- Ashutosh
Re: [Intel-gfx] [PATCH 3/9] drm/i915/pcode: Extend pcode functions for multiple gt's
On Thu, Apr 28, 2022 at 05:39:37PM -0700, Ashutosh Dixit wrote:
> Each gt contains an independent instance of pcode. Extend pcode functions
> to interface with pcode on different gt's. To avoid creating dependency of
> display functionality on intel_gt, new pcode function interfaces are
> exposed in terms of uncore rather than intel_gt. Previous struct
> drm_i915_private based pcode interfaces are preserved.
>
> v2: Expose pcode functions in terms of uncore rather than gt (Jani/Rodrigo)
thank you for that! it looks better with the uncore.
sorry for not thinking about this earlier.
but some comments below...
>
> Cc: Rodrigo Vivi
> Cc: Jani Nikula
> Cc: Andi Shyti
> Signed-off-by: Ashutosh Dixit
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 17 +++
> drivers/gpu/drm/i915/gt/intel_gt.h | 2 +
> drivers/gpu/drm/i915/i915_driver.c | 4 +-
> drivers/gpu/drm/i915/intel_pcode.c | 76 +++---
> drivers/gpu/drm/i915/intel_pcode.h | 29 +---
> 5 files changed, 80 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 92394f13b42f..07cfe66dd0e8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -28,6 +28,7 @@
> #include "intel_rps.h"
> #include "intel_gt_sysfs.h"
> #include "intel_uncore.h"
> +#include "intel_pcode.h"
> #include "shmem_utils.h"
>
> static void __intel_gt_init_early(struct intel_gt *gt)
> @@ -1240,3 +1241,19 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt)
> intel_uncore_forcewake_put_delayed(uncore, FORCEWAKE_ALL);
> mutex_unlock(>->tlb_invalidate_lock);
> }
> +
> +int intel_gt_pcode_init(struct drm_i915_private *i915)
> +{
> + struct intel_gt *gt;
> + int id, ret;
> +
> + for_each_gt(gt, i915, id) {
> + ret = intel_pcode_init(gt->uncore);
> + if (ret) {
> + drm_err(>->i915->drm, "gt %d: intel_pcode_init failed
> %d\n", id, ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h
> b/drivers/gpu/drm/i915/gt/intel_gt.h
> index 44c6cb63ccbc..241d833fdb1e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
> @@ -125,6 +125,8 @@ void intel_gt_watchdog_work(struct work_struct *work);
>
> void intel_gt_invalidate_tlbs(struct intel_gt *gt);
>
> +int intel_gt_pcode_init(struct drm_i915_private *i915);
> +
> struct resource intel_pci_resource(struct pci_dev *pdev, int bar);
>
> #endif /* __INTEL_GT_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_driver.c
> b/drivers/gpu/drm/i915/i915_driver.c
> index 90b0ce5051af..518d6e357017 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -629,7 +629,7 @@ static int i915_driver_hw_probe(struct drm_i915_private
> *dev_priv)
>
> intel_opregion_setup(dev_priv);
>
> - ret = intel_pcode_init(dev_priv);
> + ret = intel_gt_pcode_init(dev_priv);
> if (ret)
> goto err_msi;
>
> @@ -1251,7 +1251,7 @@ static int i915_drm_resume(struct drm_device *dev)
>
> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>
> - ret = intel_pcode_init(dev_priv);
> + ret = intel_gt_pcode_init(dev_priv);
I didn't like we have this indirection i915 -> gt -> i915...
At the same time I understand you don't want to duplicate the for_each with
the error msg and all in here.
So, what about having in this file a
static int __init_pcode(dev_priv)
?!
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/i915/intel_pcode.c
> b/drivers/gpu/drm/i915/intel_pcode.c
> index ac727546868e..66020b2e461f 100644
> --- a/drivers/gpu/drm/i915/intel_pcode.c
> +++ b/drivers/gpu/drm/i915/intel_pcode.c
> @@ -52,14 +52,12 @@ static int gen7_check_mailbox_status(u32 mbox)
> }
> }
>
> -static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
> +static int intel_pcode_rw(struct intel_uncore *uncore, u32 mbox,
I'm not sure if I like the idea of the renaming here...
I mean, it looks nicer indeed, but at the same time the "intel_"
make it looks it is exported one.
> u32 *val, u32 *val1,
> int fast_timeout_us, int slow_timeout_ms,
> bool is_read)
> {
> - struct intel_uncore *uncore = &i915->uncore;
> -
> - lockdep_assert_held(&i915->sb_lock);
> + lockdep_assert_held(&uncore->i915->sb_lock);
>
> /*
>* GEN6_PCODE_* are outside of the forcewake domain, we can use
> @@ -88,22 +86,22 @@ static int __snb_pcode_rw(struct drm_i915_private *i915,
> u32 mbox,
> if (is_read && val1)
> *val1 = intel_uncore_read_fw(uncore, GEN6_PCODE_DATA1);
>
> - if (GRAPHICS_VER(i915) > 6)
> + if (GRAPHICS_VER(uncore->i915) > 6)
> return gen7_check_mailbox_status(mbox);
> el
Re: [Intel-gfx] [PATCH 3/9] drm/i915/pcode: Extend pcode functions for multiple gt's
On Sun, 24 Apr 2022 12:08:18 -0700, Andi Shyti wrote:
>
> Hi Ashutosh,
Hi Andi,
> [...]
>
> > -static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
> > - u32 request, u32 reply_mask, u32 reply,
> > - u32 *status)
> > +static bool __gt_pcode_try_request(struct intel_gt *gt, u32 mbox,
>
> why is this becoming a '__' function?
Fixed in v3.
> > int intel_pcode_init(struct drm_i915_private *i915)
> > {
> > - int ret = 0;
> > + struct intel_gt *gt;
> > + int i, ret = 0;
> >
> > if (!IS_DGFX(i915))
> > return ret;
>
> we can take some freedom, if you don't mind, and declare ret
> inside the for_each, and return 0 here. Just a small cosmetic.
Good idea, changed in v3.
> > +#define skl_pcode_request(i915, mbox, request, reply_mask, reply,
> > timeout_base_ms) \
> > + intel_gt_pcode_request(&(i915)->gt0, mbox, request, reply_mask, reply,
> > timeout_base_ms)
>
> to_gt(i915)
Not needed in v3 due to interface change to uncore.
> I guess this is just a replacement i915 to gt, I think it's all
> correct and with the latter changed:
>
> Reviewed-by: Andi Shyti
I've removed the R-b from this patch due to interface change to uncore
since it's a significant change. I have retained R-b on the following
patches since those changes are just s/gt/gt->uncore/ .
Thanks.
--
Ashutosh
[Intel-gfx] [PATCH 3/9] drm/i915/pcode: Extend pcode functions for multiple gt's
Each gt contains an independent instance of pcode. Extend pcode functions
to interface with pcode on different gt's. To avoid creating dependency of
display functionality on intel_gt, new pcode function interfaces are
exposed in terms of uncore rather than intel_gt. Previous struct
drm_i915_private based pcode interfaces are preserved.
v2: Expose pcode functions in terms of uncore rather than gt (Jani/Rodrigo)
Cc: Rodrigo Vivi
Cc: Jani Nikula
Cc: Andi Shyti
Signed-off-by: Ashutosh Dixit
---
drivers/gpu/drm/i915/gt/intel_gt.c | 17 +++
drivers/gpu/drm/i915/gt/intel_gt.h | 2 +
drivers/gpu/drm/i915/i915_driver.c | 4 +-
drivers/gpu/drm/i915/intel_pcode.c | 76 +++---
drivers/gpu/drm/i915/intel_pcode.h | 29 +---
5 files changed, 80 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index 92394f13b42f..07cfe66dd0e8 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -28,6 +28,7 @@
#include "intel_rps.h"
#include "intel_gt_sysfs.h"
#include "intel_uncore.h"
+#include "intel_pcode.h"
#include "shmem_utils.h"
static void __intel_gt_init_early(struct intel_gt *gt)
@@ -1240,3 +1241,19 @@ void intel_gt_invalidate_tlbs(struct intel_gt *gt)
intel_uncore_forcewake_put_delayed(uncore, FORCEWAKE_ALL);
mutex_unlock(>->tlb_invalidate_lock);
}
+
+int intel_gt_pcode_init(struct drm_i915_private *i915)
+{
+ struct intel_gt *gt;
+ int id, ret;
+
+ for_each_gt(gt, i915, id) {
+ ret = intel_pcode_init(gt->uncore);
+ if (ret) {
+ drm_err(>->i915->drm, "gt %d: intel_pcode_init failed
%d\n", id, ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h
b/drivers/gpu/drm/i915/gt/intel_gt.h
index 44c6cb63ccbc..241d833fdb1e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -125,6 +125,8 @@ void intel_gt_watchdog_work(struct work_struct *work);
void intel_gt_invalidate_tlbs(struct intel_gt *gt);
+int intel_gt_pcode_init(struct drm_i915_private *i915);
+
struct resource intel_pci_resource(struct pci_dev *pdev, int bar);
#endif /* __INTEL_GT_H__ */
diff --git a/drivers/gpu/drm/i915/i915_driver.c
b/drivers/gpu/drm/i915/i915_driver.c
index 90b0ce5051af..518d6e357017 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -629,7 +629,7 @@ static int i915_driver_hw_probe(struct drm_i915_private
*dev_priv)
intel_opregion_setup(dev_priv);
- ret = intel_pcode_init(dev_priv);
+ ret = intel_gt_pcode_init(dev_priv);
if (ret)
goto err_msi;
@@ -1251,7 +1251,7 @@ static int i915_drm_resume(struct drm_device *dev)
disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
- ret = intel_pcode_init(dev_priv);
+ ret = intel_gt_pcode_init(dev_priv);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_pcode.c
b/drivers/gpu/drm/i915/intel_pcode.c
index ac727546868e..66020b2e461f 100644
--- a/drivers/gpu/drm/i915/intel_pcode.c
+++ b/drivers/gpu/drm/i915/intel_pcode.c
@@ -52,14 +52,12 @@ static int gen7_check_mailbox_status(u32 mbox)
}
}
-static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
+static int intel_pcode_rw(struct intel_uncore *uncore, u32 mbox,
u32 *val, u32 *val1,
int fast_timeout_us, int slow_timeout_ms,
bool is_read)
{
- struct intel_uncore *uncore = &i915->uncore;
-
- lockdep_assert_held(&i915->sb_lock);
+ lockdep_assert_held(&uncore->i915->sb_lock);
/*
* GEN6_PCODE_* are outside of the forcewake domain, we can use
@@ -88,22 +86,22 @@ static int __snb_pcode_rw(struct drm_i915_private *i915,
u32 mbox,
if (is_read && val1)
*val1 = intel_uncore_read_fw(uncore, GEN6_PCODE_DATA1);
- if (GRAPHICS_VER(i915) > 6)
+ if (GRAPHICS_VER(uncore->i915) > 6)
return gen7_check_mailbox_status(mbox);
else
return gen6_check_mailbox_status(mbox);
}
-int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32
*val1)
+int intel_pcode_read(struct intel_uncore *uncore, u32 mbox, u32 *val, u32
*val1)
{
int err;
- mutex_lock(&i915->sb_lock);
- err = __snb_pcode_rw(i915, mbox, val, val1, 500, 20, true);
- mutex_unlock(&i915->sb_lock);
+ mutex_lock(&uncore->i915->sb_lock);
+ err = intel_pcode_rw(uncore, mbox, val, val1, 500, 20, true);
+ mutex_unlock(&uncore->i915->sb_lock);
if (err) {
- drm_dbg(&i915->drm,
+ drm_dbg(&uncore->i915->drm,
"warning: pcode (read from mbox %x) mailbox access
failed for %ps: %d\n",
Re: [Intel-gfx] [PATCH 3/9] drm/i915/pcode: Extend pcode functions for multiple gt's
On Tue, 26 Apr 2022 00:55:26 -0700, Jani Nikula wrote: > > On Tue, 19 Apr 2022, Ashutosh Dixit wrote: > > Each gt contains an independent instance of pcode. Extend pcode functions > > to interface with pcode on different gt's. Previous (GT0) pcode read/write > > interfaces are preserved. > > Replying here as well. I'd prefer it if a dependency on gt wasn't > introduced here. You could just pass the uncore. This seems like a good solution, I will rework the patches. Thanks.
Re: [Intel-gfx] [PATCH 3/9] drm/i915/pcode: Extend pcode functions for multiple gt's
On Tue, 19 Apr 2022, Ashutosh Dixit wrote:
> Each gt contains an independent instance of pcode. Extend pcode functions
> to interface with pcode on different gt's. Previous (GT0) pcode read/write
> interfaces are preserved.
Replying here as well. I'd prefer it if a dependency on gt wasn't
introduced here. You could just pass the uncore.
BR,
Jani.
>
> Cc: Rodrigo Vivi
> Cc: Mike Ruhl
> Signed-off-by: Ashutosh Dixit
> ---
> drivers/gpu/drm/i915/intel_pcode.c | 108 -
> drivers/gpu/drm/i915/intel_pcode.h | 27 ++--
> 2 files changed, 82 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pcode.c
> b/drivers/gpu/drm/i915/intel_pcode.c
> index ac727546868e..0cff212cc81b 100644
> --- a/drivers/gpu/drm/i915/intel_pcode.c
> +++ b/drivers/gpu/drm/i915/intel_pcode.c
> @@ -6,6 +6,7 @@
> #include "i915_drv.h"
> #include "i915_reg.h"
> #include "intel_pcode.h"
> +#include "gt/intel_gt.h"
>
> static int gen6_check_mailbox_status(u32 mbox)
> {
> @@ -52,14 +53,14 @@ static int gen7_check_mailbox_status(u32 mbox)
> }
> }
>
> -static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
> - u32 *val, u32 *val1,
> - int fast_timeout_us, int slow_timeout_ms,
> - bool is_read)
> +static int __gt_pcode_rw(struct intel_gt *gt, u32 mbox,
> + u32 *val, u32 *val1,
> + int fast_timeout_us, int slow_timeout_ms,
> + bool is_read)
> {
> - struct intel_uncore *uncore = &i915->uncore;
> + struct intel_uncore *uncore = gt->uncore;
>
> - lockdep_assert_held(&i915->sb_lock);
> + lockdep_assert_held(>->i915->sb_lock);
>
> /*
>* GEN6_PCODE_* are outside of the forcewake domain, we can use
> @@ -88,60 +89,60 @@ static int __snb_pcode_rw(struct drm_i915_private *i915,
> u32 mbox,
> if (is_read && val1)
> *val1 = intel_uncore_read_fw(uncore, GEN6_PCODE_DATA1);
>
> - if (GRAPHICS_VER(i915) > 6)
> + if (GRAPHICS_VER(gt->i915) > 6)
> return gen7_check_mailbox_status(mbox);
> else
> return gen6_check_mailbox_status(mbox);
> }
>
> -int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32
> *val1)
> +int intel_gt_pcode_read(struct intel_gt *gt, u32 mbox, u32 *val, u32 *val1)
> {
> int err;
>
> - mutex_lock(&i915->sb_lock);
> - err = __snb_pcode_rw(i915, mbox, val, val1, 500, 20, true);
> - mutex_unlock(&i915->sb_lock);
> + mutex_lock(>->i915->sb_lock);
> + err = __gt_pcode_rw(gt, mbox, val, val1, 500, 20, true);
> + mutex_unlock(>->i915->sb_lock);
>
> if (err) {
> - drm_dbg(&i915->drm,
> - "warning: pcode (read from mbox %x) mailbox access
> failed for %ps: %d\n",
> - mbox, __builtin_return_address(0), err);
> + drm_dbg(>->i915->drm,
> + "gt %d: warning: pcode (read from mbox %x) mailbox
> access failed for %ps: %d\n",
> + gt->info.id, mbox, __builtin_return_address(0), err);
> }
>
> return err;
> }
>
> -int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
> - int fast_timeout_us, int slow_timeout_ms)
> +int intel_gt_pcode_write_timeout(struct intel_gt *gt, u32 mbox, u32 val,
> + int fast_timeout_us, int slow_timeout_ms)
> {
> int err;
>
> - mutex_lock(&i915->sb_lock);
> - err = __snb_pcode_rw(i915, mbox, &val, NULL,
> - fast_timeout_us, slow_timeout_ms, false);
> - mutex_unlock(&i915->sb_lock);
> + mutex_lock(>->i915->sb_lock);
> + err = __gt_pcode_rw(gt, mbox, &val, NULL,
> + fast_timeout_us, slow_timeout_ms, false);
> + mutex_unlock(>->i915->sb_lock);
>
> if (err) {
> - drm_dbg(&i915->drm,
> - "warning: pcode (write of 0x%08x to mbox %x) mailbox
> access failed for %ps: %d\n",
> - val, mbox, __builtin_return_address(0), err);
> + drm_dbg(>->i915->drm,
> + "gt %d: warning: pcode (write of 0x%08x to mbox %x)
> mailbox access failed for %ps: %d\n",
> + gt->info.id, val, mbox, __builtin_return_address(0),
> err);
> }
>
> return err;
> }
>
> -static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
> - u32 request, u32 reply_mask, u32 reply,
> - u32 *status)
> +static bool __gt_pcode_try_request(struct intel_gt *gt, u32 mbox,
> +u32 request, u32 reply_mask, u32 reply,
> +u32 *status)
> {
> - *status = __snb_pcode_rw(i915, mbox, &request, NULL, 500, 0, true);
> + *status = __gt_pcode_rw(gt, mbox, &request, NULL, 500, 0, true);
>
Re: [Intel-gfx] [PATCH 3/9] drm/i915/pcode: Extend pcode functions for multiple gt's
Hi Ashutosh,
[...]
> -static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
> - u32 request, u32 reply_mask, u32 reply,
> - u32 *status)
> +static bool __gt_pcode_try_request(struct intel_gt *gt, u32 mbox,
why is this becoming a '__' function?
> +u32 request, u32 reply_mask, u32 reply,
> +u32 *status)
> {
> - *status = __snb_pcode_rw(i915, mbox, &request, NULL, 500, 0, true);
> + *status = __gt_pcode_rw(gt, mbox, &request, NULL, 500, 0, true);
>
> return (*status == 0) && ((request & reply_mask) == reply);
> }
>
> /**
> - * skl_pcode_request - send PCODE request until acknowledgment
> - * @i915: device private
> + * intel_gt_pcode_request - send PCODE request until acknowledgment
> + * @gt: gt
> * @mbox: PCODE mailbox ID the request is targeted for
> * @request: request ID
> * @reply_mask: mask used to check for request acknowledgment
> @@ -158,16 +159,16 @@ static bool skl_pcode_try_request(struct
> drm_i915_private *i915, u32 mbox,
> * Returns 0 on success, %-ETIMEDOUT in case of a timeout, <0 in case of some
> * other error as reported by PCODE.
> */
> -int skl_pcode_request(struct drm_i915_private *i915, u32 mbox, u32 request,
> - u32 reply_mask, u32 reply, int timeout_base_ms)
> +int intel_gt_pcode_request(struct intel_gt *gt, u32 mbox, u32 request,
> +u32 reply_mask, u32 reply, int timeout_base_ms)
> {
> u32 status;
> int ret;
>
> - mutex_lock(&i915->sb_lock);
> + mutex_lock(>->i915->sb_lock);
>
> #define COND \
> - skl_pcode_try_request(i915, mbox, request, reply_mask, reply, &status)
> + __gt_pcode_try_request(gt, mbox, request, reply_mask, reply, &status)
>
> /*
>* Prime the PCODE by doing a request first. Normally it guarantees
> @@ -193,35 +194,48 @@ int skl_pcode_request(struct drm_i915_private *i915,
> u32 mbox, u32 request,
>* requests, and for any quirks of the PCODE firmware that delays
>* the request completion.
>*/
> - drm_dbg_kms(&i915->drm,
> + drm_dbg_kms(>->i915->drm,
> "PCODE timeout, retrying with preemption disabled\n");
> - drm_WARN_ON_ONCE(&i915->drm, timeout_base_ms > 3);
> + drm_WARN_ON_ONCE(>->i915->drm, timeout_base_ms > 3);
> preempt_disable();
> ret = wait_for_atomic(COND, 50);
> preempt_enable();
>
> out:
> - mutex_unlock(&i915->sb_lock);
> + mutex_unlock(>->i915->sb_lock);
> return status ? status : ret;
> #undef COND
> }
>
> +static int __gt_pcode_init(struct intel_gt *gt)
> +{
> + int ret = intel_gt_pcode_request(gt, DG1_PCODE_STATUS,
> + DG1_UNCORE_GET_INIT_STATUS,
> + DG1_UNCORE_INIT_STATUS_COMPLETE,
> + DG1_UNCORE_INIT_STATUS_COMPLETE,
> 18);
> +
> + drm_dbg(>->i915->drm, "gt %d: PCODE init status %d\n", gt->info.id,
> ret);
> +
> + if (ret)
> + drm_err(>->i915->drm, "gt %d: Pcode did not report uncore
> initialization completion!\n",
> + gt->info.id);
> +
> + return ret;
> +}
> +
> int intel_pcode_init(struct drm_i915_private *i915)
> {
> - int ret = 0;
> + struct intel_gt *gt;
> + int i, ret = 0;
>
> if (!IS_DGFX(i915))
> return ret;
we can take some freedom, if you don't mind, and declare ret
inside the for_each, and return 0 here. Just a small cosmetic.
>
> - ret = skl_pcode_request(i915, DG1_PCODE_STATUS,
> - DG1_UNCORE_GET_INIT_STATUS,
> - DG1_UNCORE_INIT_STATUS_COMPLETE,
> - DG1_UNCORE_INIT_STATUS_COMPLETE, 18);
> -
> - drm_dbg(&i915->drm, "PCODE init status %d\n", ret);
> -
> - if (ret)
> - drm_err(&i915->drm, "Pcode did not report uncore initialization
> completion!\n");
> + for_each_gt(gt, i915, i) {
> + ret = __gt_pcode_init(gt);
> + if (ret)
> + return ret;
> + }
>
> - return ret;
> + return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_pcode.h
> b/drivers/gpu/drm/i915/intel_pcode.h
> index 0962a17fac48..96c954ec91f9 100644
> --- a/drivers/gpu/drm/i915/intel_pcode.h
> +++ b/drivers/gpu/drm/i915/intel_pcode.h
> @@ -8,16 +8,31 @@
>
> #include
>
> +struct intel_gt;
> struct drm_i915_private;
>
> -int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32
> *val1);
> -int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
> - int fast_timeout_us, int slow_timeout_ms);
> -#define snb_pcode_write(i915, mbox, val) \
> +int intel_gt_pcode_read(struct intel_gt *gt, u32 mbox, u32 *val, u32 *val1);
> +
> +int intel_gt_pcode_write_t
[Intel-gfx] [PATCH 3/9] drm/i915/pcode: Extend pcode functions for multiple gt's
Each gt contains an independent instance of pcode. Extend pcode functions
to interface with pcode on different gt's. Previous (GT0) pcode read/write
interfaces are preserved.
Cc: Rodrigo Vivi
Cc: Mike Ruhl
Signed-off-by: Ashutosh Dixit
---
drivers/gpu/drm/i915/intel_pcode.c | 108 -
drivers/gpu/drm/i915/intel_pcode.h | 27 ++--
2 files changed, 82 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pcode.c
b/drivers/gpu/drm/i915/intel_pcode.c
index ac727546868e..0cff212cc81b 100644
--- a/drivers/gpu/drm/i915/intel_pcode.c
+++ b/drivers/gpu/drm/i915/intel_pcode.c
@@ -6,6 +6,7 @@
#include "i915_drv.h"
#include "i915_reg.h"
#include "intel_pcode.h"
+#include "gt/intel_gt.h"
static int gen6_check_mailbox_status(u32 mbox)
{
@@ -52,14 +53,14 @@ static int gen7_check_mailbox_status(u32 mbox)
}
}
-static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
- u32 *val, u32 *val1,
- int fast_timeout_us, int slow_timeout_ms,
- bool is_read)
+static int __gt_pcode_rw(struct intel_gt *gt, u32 mbox,
+u32 *val, u32 *val1,
+int fast_timeout_us, int slow_timeout_ms,
+bool is_read)
{
- struct intel_uncore *uncore = &i915->uncore;
+ struct intel_uncore *uncore = gt->uncore;
- lockdep_assert_held(&i915->sb_lock);
+ lockdep_assert_held(>->i915->sb_lock);
/*
* GEN6_PCODE_* are outside of the forcewake domain, we can use
@@ -88,60 +89,60 @@ static int __snb_pcode_rw(struct drm_i915_private *i915,
u32 mbox,
if (is_read && val1)
*val1 = intel_uncore_read_fw(uncore, GEN6_PCODE_DATA1);
- if (GRAPHICS_VER(i915) > 6)
+ if (GRAPHICS_VER(gt->i915) > 6)
return gen7_check_mailbox_status(mbox);
else
return gen6_check_mailbox_status(mbox);
}
-int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32
*val1)
+int intel_gt_pcode_read(struct intel_gt *gt, u32 mbox, u32 *val, u32 *val1)
{
int err;
- mutex_lock(&i915->sb_lock);
- err = __snb_pcode_rw(i915, mbox, val, val1, 500, 20, true);
- mutex_unlock(&i915->sb_lock);
+ mutex_lock(>->i915->sb_lock);
+ err = __gt_pcode_rw(gt, mbox, val, val1, 500, 20, true);
+ mutex_unlock(>->i915->sb_lock);
if (err) {
- drm_dbg(&i915->drm,
- "warning: pcode (read from mbox %x) mailbox access
failed for %ps: %d\n",
- mbox, __builtin_return_address(0), err);
+ drm_dbg(>->i915->drm,
+ "gt %d: warning: pcode (read from mbox %x) mailbox
access failed for %ps: %d\n",
+ gt->info.id, mbox, __builtin_return_address(0), err);
}
return err;
}
-int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
- int fast_timeout_us, int slow_timeout_ms)
+int intel_gt_pcode_write_timeout(struct intel_gt *gt, u32 mbox, u32 val,
+int fast_timeout_us, int slow_timeout_ms)
{
int err;
- mutex_lock(&i915->sb_lock);
- err = __snb_pcode_rw(i915, mbox, &val, NULL,
-fast_timeout_us, slow_timeout_ms, false);
- mutex_unlock(&i915->sb_lock);
+ mutex_lock(>->i915->sb_lock);
+ err = __gt_pcode_rw(gt, mbox, &val, NULL,
+ fast_timeout_us, slow_timeout_ms, false);
+ mutex_unlock(>->i915->sb_lock);
if (err) {
- drm_dbg(&i915->drm,
- "warning: pcode (write of 0x%08x to mbox %x) mailbox
access failed for %ps: %d\n",
- val, mbox, __builtin_return_address(0), err);
+ drm_dbg(>->i915->drm,
+ "gt %d: warning: pcode (write of 0x%08x to mbox %x)
mailbox access failed for %ps: %d\n",
+ gt->info.id, val, mbox, __builtin_return_address(0),
err);
}
return err;
}
-static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
- u32 request, u32 reply_mask, u32 reply,
- u32 *status)
+static bool __gt_pcode_try_request(struct intel_gt *gt, u32 mbox,
+ u32 request, u32 reply_mask, u32 reply,
+ u32 *status)
{
- *status = __snb_pcode_rw(i915, mbox, &request, NULL, 500, 0, true);
+ *status = __gt_pcode_rw(gt, mbox, &request, NULL, 500, 0, true);
return (*status == 0) && ((request & reply_mask) == reply);
}
/**
- * skl_pcode_request - send PCODE request until acknowledgment
- * @i915: device private
+ * intel_gt_pcode_request - send PCODE request until acknowledgment
+ * @gt: gt
* @mbox: PCODE mailbox ID the request is tar
[Intel-gfx] [PATCH 3/9] drm/i915/pcode: Extend pcode functions for multiple gt's
Each gt contains an independent instance of pcode. Extend pcode functions
to interface with pcode on different gt's. Previous (GT0) pcode read/write
interfaces are preserved.
Cc: Rodrigo Vivi
Cc: Mike Ruhl
Signed-off-by: Ashutosh Dixit
---
drivers/gpu/drm/i915/intel_pcode.c | 108 -
drivers/gpu/drm/i915/intel_pcode.h | 27 ++--
2 files changed, 82 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pcode.c
b/drivers/gpu/drm/i915/intel_pcode.c
index ac727546868e..0cff212cc81b 100644
--- a/drivers/gpu/drm/i915/intel_pcode.c
+++ b/drivers/gpu/drm/i915/intel_pcode.c
@@ -6,6 +6,7 @@
#include "i915_drv.h"
#include "i915_reg.h"
#include "intel_pcode.h"
+#include "gt/intel_gt.h"
static int gen6_check_mailbox_status(u32 mbox)
{
@@ -52,14 +53,14 @@ static int gen7_check_mailbox_status(u32 mbox)
}
}
-static int __snb_pcode_rw(struct drm_i915_private *i915, u32 mbox,
- u32 *val, u32 *val1,
- int fast_timeout_us, int slow_timeout_ms,
- bool is_read)
+static int __gt_pcode_rw(struct intel_gt *gt, u32 mbox,
+u32 *val, u32 *val1,
+int fast_timeout_us, int slow_timeout_ms,
+bool is_read)
{
- struct intel_uncore *uncore = &i915->uncore;
+ struct intel_uncore *uncore = gt->uncore;
- lockdep_assert_held(&i915->sb_lock);
+ lockdep_assert_held(>->i915->sb_lock);
/*
* GEN6_PCODE_* are outside of the forcewake domain, we can use
@@ -88,60 +89,60 @@ static int __snb_pcode_rw(struct drm_i915_private *i915,
u32 mbox,
if (is_read && val1)
*val1 = intel_uncore_read_fw(uncore, GEN6_PCODE_DATA1);
- if (GRAPHICS_VER(i915) > 6)
+ if (GRAPHICS_VER(gt->i915) > 6)
return gen7_check_mailbox_status(mbox);
else
return gen6_check_mailbox_status(mbox);
}
-int snb_pcode_read(struct drm_i915_private *i915, u32 mbox, u32 *val, u32
*val1)
+int intel_gt_pcode_read(struct intel_gt *gt, u32 mbox, u32 *val, u32 *val1)
{
int err;
- mutex_lock(&i915->sb_lock);
- err = __snb_pcode_rw(i915, mbox, val, val1, 500, 20, true);
- mutex_unlock(&i915->sb_lock);
+ mutex_lock(>->i915->sb_lock);
+ err = __gt_pcode_rw(gt, mbox, val, val1, 500, 20, true);
+ mutex_unlock(>->i915->sb_lock);
if (err) {
- drm_dbg(&i915->drm,
- "warning: pcode (read from mbox %x) mailbox access
failed for %ps: %d\n",
- mbox, __builtin_return_address(0), err);
+ drm_dbg(>->i915->drm,
+ "gt %d: warning: pcode (read from mbox %x) mailbox
access failed for %ps: %d\n",
+ gt->info.id, mbox, __builtin_return_address(0), err);
}
return err;
}
-int snb_pcode_write_timeout(struct drm_i915_private *i915, u32 mbox, u32 val,
- int fast_timeout_us, int slow_timeout_ms)
+int intel_gt_pcode_write_timeout(struct intel_gt *gt, u32 mbox, u32 val,
+int fast_timeout_us, int slow_timeout_ms)
{
int err;
- mutex_lock(&i915->sb_lock);
- err = __snb_pcode_rw(i915, mbox, &val, NULL,
-fast_timeout_us, slow_timeout_ms, false);
- mutex_unlock(&i915->sb_lock);
+ mutex_lock(>->i915->sb_lock);
+ err = __gt_pcode_rw(gt, mbox, &val, NULL,
+ fast_timeout_us, slow_timeout_ms, false);
+ mutex_unlock(>->i915->sb_lock);
if (err) {
- drm_dbg(&i915->drm,
- "warning: pcode (write of 0x%08x to mbox %x) mailbox
access failed for %ps: %d\n",
- val, mbox, __builtin_return_address(0), err);
+ drm_dbg(>->i915->drm,
+ "gt %d: warning: pcode (write of 0x%08x to mbox %x)
mailbox access failed for %ps: %d\n",
+ gt->info.id, val, mbox, __builtin_return_address(0),
err);
}
return err;
}
-static bool skl_pcode_try_request(struct drm_i915_private *i915, u32 mbox,
- u32 request, u32 reply_mask, u32 reply,
- u32 *status)
+static bool __gt_pcode_try_request(struct intel_gt *gt, u32 mbox,
+ u32 request, u32 reply_mask, u32 reply,
+ u32 *status)
{
- *status = __snb_pcode_rw(i915, mbox, &request, NULL, 500, 0, true);
+ *status = __gt_pcode_rw(gt, mbox, &request, NULL, 500, 0, true);
return (*status == 0) && ((request & reply_mask) == reply);
}
/**
- * skl_pcode_request - send PCODE request until acknowledgment
- * @i915: device private
+ * intel_gt_pcode_request - send PCODE request until acknowledgment
+ * @gt: gt
* @mbox: PCODE mailbox ID the request is tar
