[PATCH 1/2] drm/i915: Stop lying about the WOPCM size

[PATCH 1/2] drm/i915: Stop lying about the WOPCM size

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Most plattforms don't have a fixed 1MiB WOPCM so stop saying that they
do.

Also toss in a FIXME about actually using the WOPCM size we probed from
the hardware instead of assuming the fixed 1MiB size.

Cc: Jackie Li <yaodong.li@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_wopcm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index 74bf76f3fddc..75c7a2b0c869 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -71,6 +71,12 @@
  */
 void intel_wopcm_init_early(struct intel_wopcm *wopcm)
 {
+	struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
+
+	if (!HAS_GUC(i915))
+		return;
+
+	/* FIXME use the size we actually probed from the hardware */
 	wopcm->size = GEN9_WOPCM_SIZE;
 
 	DRM_DEBUG_DRIVER("WOPCM size: %uKiB\n", wopcm->size / 1024);
@@ -163,7 +169,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
 	u32 guc_wopcm_rsvd;
 	int err;
 
-	GEM_BUG_ON(!wopcm->size);
+	if (!wopcm->size)
+		return 0;
 
 	if (guc_fw_size >= wopcm->size) {
 		DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",
-- 
2.16.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Sanitize enable_guc properly on non-guc platforms

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If there's no guc don't try to initialize it even if the user asked for
it.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 7c95697e1a35..2765808b01e0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -106,6 +106,11 @@ static void sanitize_options_early(struct drm_i915_private *i915)
 	struct intel_uc_fw *guc_fw = &i915->guc.fw;
 	struct intel_uc_fw *huc_fw = &i915->huc.fw;
 
+	if (!HAS_GUC(i915)) {
+		i915_modparams.enable_guc = 0;
+		return;
+	}
+
 	/* A negative value means "use platform default" */
 	if (i915_modparams.enable_guc < 0)
 		i915_modparams.enable_guc = __get_platform_enable_guc(i915);
-- 
2.16.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Stop lying about the WOPCM size

[Chris Wilson]

Quoting Ville Syrjala (2018-07-17 13:53:19)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Most plattforms don't have a fixed 1MiB WOPCM so stop saying that they
> do.
> 
> Also toss in a FIXME about actually using the WOPCM size we probed from
> the hardware instead of assuming the fixed 1MiB size.
> 
> Cc: Jackie Li <yaodong.li@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>

Michał wrote a near identical patch

> ---
>  drivers/gpu/drm/i915/intel_wopcm.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
> index 74bf76f3fddc..75c7a2b0c869 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -71,6 +71,12 @@
>   */
>  void intel_wopcm_init_early(struct intel_wopcm *wopcm)
>  {
> +       struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> +
> +       if (!HAS_GUC(i915))
> +               return;
> +
> +       /* FIXME use the size we actually probed from the hardware */
>         wopcm->size = GEN9_WOPCM_SIZE;
>  
>         DRM_DEBUG_DRIVER("WOPCM size: %uKiB\n", wopcm->size / 1024);
> @@ -163,7 +169,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
>         u32 guc_wopcm_rsvd;
>         int err;
>  
> -       GEM_BUG_ON(!wopcm->size);
> +       if (!wopcm->size)
> +               return 0;

...except he chose to keep the GEM_BUG_ON. My personal preference would
be to use the driver value (wopcm->size) here, so 

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

but if Michał et al feel strongly that they would rather keep the !size
sanity check, they need to speak up now :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Sanitize enable_guc properly on non-guc platforms

[Chris Wilson]

Quoting Ville Syrjala (2018-07-17 13:53:20)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> If there's no guc don't try to initialize it even if the user asked for
> it.
> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I like it for less debug spam, so suits me (I might even wish for greater
reductions in noise about unused fw...)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Sanitize enable_guc properly on non-guc platforms

[Michal Wajdeczko]

On Tue, 17 Jul 2018 14:53:20 +0200, Ville Syrjala  
<ville.syrjala@linux.intel.com> wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If there's no guc don't try to initialize it even if the user asked for
> it.
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uc.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 7c95697e1a35..2765808b01e0 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -106,6 +106,11 @@ static void sanitize_options_early(struct  
> drm_i915_private *i915)
>  	struct intel_uc_fw *guc_fw = &i915->guc.fw;
>  	struct intel_uc_fw *huc_fw = &i915->huc.fw;
> +	if (!HAS_GUC(i915)) {
> +		i915_modparams.enable_guc = 0;
> +		return;
> +	}
> +

This will silently switch from user requested GuC-submission to
execlist-mode which we wanted to stop.

If user don't know what is available on given platform then auto(-1)
mode should be used instead. If user has decided to explicitly specify
invalid enable_guc !0 mode on non-GuC platform why do we want to ignore
that and continue as nothing happened?

Michal

ps. what is your expectation if there is GuC HW but no FW was defined?

>  	/* A negative value means "use platform default" */
>  	if (i915_modparams.enable_guc < 0)
>  		i915_modparams.enable_guc = __get_platform_enable_guc(i915);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Stop lying about the WOPCM size

[Michal Wajdeczko]

On Tue, 17 Jul 2018 14:53:19 +0200, Ville Syrjala  
<ville.syrjala@linux.intel.com> wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Most plattforms don't have a fixed 1MiB WOPCM so stop saying that they
> do.
>
> Also toss in a FIXME about actually using the WOPCM size we probed from
> the hardware instead of assuming the fixed 1MiB size.
>
> Cc: Jackie Li <yaodong.li@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_wopcm.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c  
> b/drivers/gpu/drm/i915/intel_wopcm.c
> index 74bf76f3fddc..75c7a2b0c869 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -71,6 +71,12 @@
>   */
>  void intel_wopcm_init_early(struct intel_wopcm *wopcm)
>  {
> +	struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> +
> +	if (!HAS_GUC(i915))
> +		return;
> +
> +	/* FIXME use the size we actually probed from the hardware */
>  	wopcm->size = GEN9_WOPCM_SIZE;
> 	DRM_DEBUG_DRIVER("WOPCM size: %uKiB\n", wopcm->size / 1024);
> @@ -163,7 +169,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
>  	u32 guc_wopcm_rsvd;
>  	int err;
> -	GEM_BUG_ON(!wopcm->size);
> +	if (!wopcm->size)
> +		return 0;

Maybe better option would be to use:

	if (!HAS_GUC(i915))
		return 0;

which will match conditions used in init_early and init_hw and
then we will also allow to run remaining detailed checks ...

> 	if (guc_fw_size >= wopcm->size) {
>  		DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",

Thanks,
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Stop lying about the WOPCM size

[Michał Winiarski]

On Tue, Jul 17, 2018 at 03:53:19PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Most plattforms don't have a fixed 1MiB WOPCM so stop saying that they
> do.
> 
> Also toss in a FIXME about actually using the WOPCM size we probed from
> the hardware instead of assuming the fixed 1MiB size.
> 
> Cc: Jackie Li <yaodong.li@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_wopcm.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
> index 74bf76f3fddc..75c7a2b0c869 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -71,6 +71,12 @@
>   */
>  void intel_wopcm_init_early(struct intel_wopcm *wopcm)
>  {
> +	struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> +
> +	if (!HAS_GUC(i915))
> +		return;

Single use dev_priv, drop the local?

> +
> +	/* FIXME use the size we actually probed from the hardware */
>  	wopcm->size = GEN9_WOPCM_SIZE;

I don't think that's exposed to us in any way.
I'd drop the FIXME - with that:

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

>  
>  	DRM_DEBUG_DRIVER("WOPCM size: %uKiB\n", wopcm->size / 1024);
> @@ -163,7 +169,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
>  	u32 guc_wopcm_rsvd;
>  	int err;
>  
> -	GEM_BUG_ON(!wopcm->size);
> +	if (!wopcm->size)
> +		return 0;
>  
>  	if (guc_fw_size >= wopcm->size) {
>  		DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Sanitize enable_guc properly on non-guc platforms

[Ville Syrjälä]

On Tue, Jul 17, 2018 at 03:26:18PM +0200, Michal Wajdeczko wrote:
> On Tue, 17 Jul 2018 14:53:20 +0200, Ville Syrjala  
> <ville.syrjala@linux.intel.com> wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > If there's no guc don't try to initialize it even if the user asked for
> > it.
> >
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_uc.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> > b/drivers/gpu/drm/i915/intel_uc.c
> > index 7c95697e1a35..2765808b01e0 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > @@ -106,6 +106,11 @@ static void sanitize_options_early(struct  
> > drm_i915_private *i915)
> >  	struct intel_uc_fw *guc_fw = &i915->guc.fw;
> >  	struct intel_uc_fw *huc_fw = &i915->huc.fw;
> > +	if (!HAS_GUC(i915)) {
> > +		i915_modparams.enable_guc = 0;
> > +		return;
> > +	}
> > +
> 
> This will silently switch from user requested GuC-submission to
> execlist-mode which we wanted to stop.
> 
> If user don't know what is available on given platform then auto(-1)
> mode should be used instead. If user has decided to explicitly specify
> invalid enable_guc !0 mode on non-GuC platform why do we want to ignore
> that and continue as nothing happened?

If we want to fail then we should at least fail nicer and tell the user
they're trying something that's not possible.

> 
> Michal
> 
> ps. what is your expectation if there is GuC HW but no FW was defined?

I just bury my head in the sand whenever a guc approaches.

> 
> >  	/* A negative value means "use platform default" */
> >  	if (i915_modparams.enable_guc < 0)
> >  		i915_modparams.enable_guc = __get_platform_enable_guc(i915);

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Stop lying about the WOPCM size

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Stop lying about the WOPCM size
URL   : https://patchwork.freedesktop.org/series/46700/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4501 -> Patchwork_9691 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46700/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9691:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-cfl-8109u}:     NOTRUN -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in Patchwork_9691 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         PASS -> DMESG-FAIL (fdo#107174)

    igt@kms_flip@basic-flip-vs-dpms:
      fi-skl-6700hq:      PASS -> DMESG-WARN (fdo#105998)

    igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence:
      fi-skl-6700k2:      PASS -> FAIL (fdo#103191)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128, fdo#107139) -> PASS

    igt@kms_busy@basic-flip-b:
      fi-skl-6700hq:      DMESG-WARN (fdo#105998) -> PASS +1

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174


== Participating hosts (46 -> 40) ==

  Additional (1): fi-cfl-8109u 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-skl-iommu fi-kbl-8809g 


== Build changes ==

    * Linux: CI_DRM_4501 -> Patchwork_9691

  CI_DRM_4501: 692d13f7b75baf0bb8c58b9784569c52d68f01e2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4559: 6d341aac2124836443ce74e8e97a4508ac8d5095 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9691: 1cb486ce2564e720b459b4c7baf4b8526098750e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1cb486ce2564 drm/i915: Sanitize enable_guc properly on non-guc platforms
20dc69bae3f7 drm/i915: Stop lying about the WOPCM size

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9691/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Sanitize enable_guc properly on non-guc platforms

[Michal Wajdeczko]

On Tue, 17 Jul 2018 16:22:01 +0200, Ville Syrjälä  
<ville.syrjala@linux.intel.com> wrote:

> On Tue, Jul 17, 2018 at 03:26:18PM +0200, Michal Wajdeczko wrote:
>> On Tue, 17 Jul 2018 14:53:20 +0200, Ville Syrjala
>> <ville.syrjala@linux.intel.com> wrote:
>>
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > If there's no guc don't try to initialize it even if the user asked  
>> for
>> > it.
>> >
>> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_uc.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_uc.c
>> > b/drivers/gpu/drm/i915/intel_uc.c
>> > index 7c95697e1a35..2765808b01e0 100644
>> > --- a/drivers/gpu/drm/i915/intel_uc.c
>> > +++ b/drivers/gpu/drm/i915/intel_uc.c
>> > @@ -106,6 +106,11 @@ static void sanitize_options_early(struct
>> > drm_i915_private *i915)
>> >  	struct intel_uc_fw *guc_fw = &i915->guc.fw;
>> >  	struct intel_uc_fw *huc_fw = &i915->huc.fw;
>> > +	if (!HAS_GUC(i915)) {
>> > +		i915_modparams.enable_guc = 0;
>> > +		return;
>> > +	}
>> > +
>>
>> This will silently switch from user requested GuC-submission to
>> execlist-mode which we wanted to stop.
>>
>> If user don't know what is available on given platform then auto(-1)
>> mode should be used instead. If user has decided to explicitly specify
>> invalid enable_guc !0 mode on non-GuC platform why do we want to ignore
>> that and continue as nothing happened?
>
> If we want to fail then we should at least fail nicer and tell the user
> they're trying something that's not possible.
>

One should see this warning message:

	DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
		 "enable_guc", i915_modparams.enable_guc,
		 !HAS_GUC(i915) ? "no GuC hardware" :
				  "no GuC firmware");

and after we finally fix -EIO support it will be followed by:

	"Failed to initialize GPU, declaring it wedged!\n"

Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Stop lying about the WOPCM size

[Michał Winiarski]

On Tue, Jul 17, 2018 at 03:52:41PM +0200, Michał Winiarski wrote:
> On Tue, Jul 17, 2018 at 03:53:19PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Most plattforms don't have a fixed 1MiB WOPCM so stop saying that they
> > do.
> > 
> > Also toss in a FIXME about actually using the WOPCM size we probed from
> > the hardware instead of assuming the fixed 1MiB size.
> > 
> > Cc: Jackie Li <yaodong.li@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_wopcm.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
> > index 74bf76f3fddc..75c7a2b0c869 100644
> > --- a/drivers/gpu/drm/i915/intel_wopcm.c
> > +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> > @@ -71,6 +71,12 @@
> >   */
> >  void intel_wopcm_init_early(struct intel_wopcm *wopcm)
> >  {
> > +	struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> > +
> > +	if (!HAS_GUC(i915))
> > +		return;
> 
> Single use dev_priv, drop the local?
> 
> > +
> > +	/* FIXME use the size we actually probed from the hardware */
> >  	wopcm->size = GEN9_WOPCM_SIZE;
> 
> I don't think that's exposed to us in any way.
> I'd drop the FIXME - with that:

Ok - I was wrong, keep the fixme.

> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> 
> -Michał
> 
> >  
> >  	DRM_DEBUG_DRIVER("WOPCM size: %uKiB\n", wopcm->size / 1024);
> > @@ -163,7 +169,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
> >  	u32 guc_wopcm_rsvd;
> >  	int err;
> >  
> > -	GEM_BUG_ON(!wopcm->size);
> > +	if (!wopcm->size)
> > +		return 0;

But I'd also go with HAS_GUC and keep the BUG_ON as Michał suggested.

-Michał

> >  
> >  	if (guc_fw_size >= wopcm->size) {
> >  		DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",
> > -- 
> > 2.16.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Stop lying about the WOPCM size

[Jackie Li]

On 07/17/2018 05:53 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Most plattforms don't have a fixed 1MiB WOPCM so stop saying that they
> do.
>
> Also toss in a FIXME about actually using the WOPCM size we probed from
> the hardware instead of assuming the fixed 1MiB size.
>
> Cc: Jackie Li <yaodong.li@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_wopcm.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
> index 74bf76f3fddc..75c7a2b0c869 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -71,6 +71,12 @@
>    */
>   void intel_wopcm_init_early(struct intel_wopcm *wopcm)
>   {
> +	struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> +
> +	if (!HAS_GUC(i915))
> +		return;
> +
> +	/* FIXME use the size we actually probed from the hardware */
>   	wopcm->size = GEN9_WOPCM_SIZE;
>   
>   	DRM_DEBUG_DRIVER("WOPCM size: %uKiB\n", wopcm->size / 1024);
> @@ -163,7 +169,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
>   	u32 guc_wopcm_rsvd;
>   	int err;
>   
> -	GEM_BUG_ON(!wopcm->size);
> +	if (!wopcm->size)
> +		return 0;
>   
>   	if (guc_fw_size >= wopcm->size) {
>   		DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",

The patch looks fine to me.

Reviewed-by: Jackie Li <yaodong.li@intel.com>

Regards,
-Jackie
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Stop lying about the WOPCM size

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Stop lying about the WOPCM size
URL   : https://patchwork.freedesktop.org/series/46700/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4501_full -> Patchwork_9691_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9691_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9691_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9691_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-render:
      shard-kbl:          PASS -> SKIP

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_9691_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_partial_pwrite_pread@writes-after-reads:
      shard-glk:          PASS -> INCOMPLETE (k.org#198133, fdo#103359)

    igt@kms_busy@basic-modeset-b:
      shard-kbl:          PASS -> DMESG-WARN (fdo#106247)

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_vblank@pipe-a-accuracy-idle:
      shard-glk:          PASS -> FAIL (fdo#102583)

    
    ==== Possible fixes ====

    igt@kms_flip@plain-flip-fb-recreate:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@perf_pmu@rc6-runtime-pm-long:
      shard-hsw:          FAIL (fdo#105010) -> PASS

    
    ==== Warnings ====

    igt@kms_sysfs_edid_timing:
      shard-hsw:          WARN (fdo#100047) -> FAIL (fdo#100047)

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010
  fdo#106247 https://bugs.freedesktop.org/show_bug.cgi?id=106247
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4501 -> Patchwork_9691

  CI_DRM_4501: 692d13f7b75baf0bb8c58b9784569c52d68f01e2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4559: 6d341aac2124836443ce74e8e97a4508ac8d5095 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9691: 1cb486ce2564e720b459b4c7baf4b8526098750e @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9691/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx