All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array
@ 2016-10-25  0:28 Anusha Srivatsa
  2016-10-25  0:46 ` ✗ Fi.CI.BAT: warning for " Patchwork
  2016-10-25 18:00 ` [PATCH] " Vivi, Rodrigo
  0 siblings, 2 replies; 6+ messages in thread
From: Anusha Srivatsa @ 2016-10-25  0:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

Currently, for display there is only one DMC image for KBL.
Remove the stepping_info table for KBL and use the no_stepping_info
array for loading the firmware.

v2: Removed the block of code as pointed out by Rodrigo to make the
loads as generic as possible.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_csr.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 1ea0e1f..d7a04bc 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -168,12 +168,6 @@ struct stepping_info {
 	char substepping;
 };
 
-static const struct stepping_info kbl_stepping_info[] = {
-	{'A', '0'}, {'B', '0'}, {'C', '0'},
-	{'D', '0'}, {'E', '0'}, {'F', '0'},
-	{'G', '0'}, {'H', '0'}, {'I', '0'},
-};
-
 static const struct stepping_info skl_stepping_info[] = {
 	{'A', '0'}, {'B', '0'}, {'C', '0'},
 	{'D', '0'}, {'E', '0'}, {'F', '0'},
@@ -194,10 +188,7 @@ intel_get_stepping_info(struct drm_i915_private *dev_priv)
 	const struct stepping_info *si;
 	unsigned int size;
 
-	if (IS_KABYLAKE(dev_priv)) {
-		size = ARRAY_SIZE(kbl_stepping_info);
-		si = kbl_stepping_info;
-	} else if (IS_SKYLAKE(dev_priv)) {
+	if (IS_SKYLAKE(dev_priv)) {
 		size = ARRAY_SIZE(skl_stepping_info);
 		si = skl_stepping_info;
 	} else if (IS_BROXTON(dev_priv)) {
-- 
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] 6+ messages in thread

* ✗ Fi.CI.BAT: warning for drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array
  2016-10-25  0:28 [PATCH] drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array Anusha Srivatsa
@ 2016-10-25  0:46 ` Patchwork
  2016-10-25 18:00 ` [PATCH] " Vivi, Rodrigo
  1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-10-25  0:46 UTC (permalink / raw)
  To: Anusha Srivatsa; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array
URL   : https://patchwork.freedesktop.org/series/14311/
State : warning

== Summary ==

Series 14311v1 drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array
https://patchwork.freedesktop.org/api/1.0/series/14311/revisions/1/mbox/

Test drv_module_reload_basic:
                skip       -> PASS       (fi-skl-6770hq)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-ivb-3770)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:246  pass:220  dwarn:1   dfail:0   fail:0   skip:25 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:219  dwarn:4   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 

Results at /Patchwork_2802/

194359e4a31ff988c7a290093820c5ef28d3752b drm-intel-nightly: 2016y-10m-24d-19h-42m-14s UTC integration manifest
5382799 drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array
  2016-10-25  0:28 [PATCH] drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array Anusha Srivatsa
  2016-10-25  0:46 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2016-10-25 18:00 ` Vivi, Rodrigo
  2016-10-26 12:32   ` Imre Deak
  1 sibling, 1 reply; 6+ messages in thread
From: Vivi, Rodrigo @ 2016-10-25 18:00 UTC (permalink / raw)
  To: Srivatsa, Anusha; +Cc: intel-gfx

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

On Mon, 2016-10-24 at 17:28 -0700, Anusha Srivatsa wrote:
> Currently, for display there is only one DMC image for KBL.
> Remove the stepping_info table for KBL and use the no_stepping_info
> array for loading the firmware.
> 
> v2: Removed the block of code as pointed out by Rodrigo to make the
> loads as generic as possible.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 1ea0e1f..d7a04bc 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -168,12 +168,6 @@ struct stepping_info {
>  	char substepping;
>  };
>  
> -static const struct stepping_info kbl_stepping_info[] = {
> -	{'A', '0'}, {'B', '0'}, {'C', '0'},
> -	{'D', '0'}, {'E', '0'}, {'F', '0'},
> -	{'G', '0'}, {'H', '0'}, {'I', '0'},
> -};
> -
>  static const struct stepping_info skl_stepping_info[] = {
>  	{'A', '0'}, {'B', '0'}, {'C', '0'},
>  	{'D', '0'}, {'E', '0'}, {'F', '0'},
> @@ -194,10 +188,7 @@ intel_get_stepping_info(struct drm_i915_private *dev_priv)
>  	const struct stepping_info *si;
>  	unsigned int size;
>  
> -	if (IS_KABYLAKE(dev_priv)) {
> -		size = ARRAY_SIZE(kbl_stepping_info);
> -		si = kbl_stepping_info;
> -	} else if (IS_SKYLAKE(dev_priv)) {
> +	if (IS_SKYLAKE(dev_priv)) {
>  		size = ARRAY_SIZE(skl_stepping_info);
>  		si = skl_stepping_info;
>  	} else if (IS_BROXTON(dev_priv)) {

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array
  2016-10-25 18:00 ` [PATCH] " Vivi, Rodrigo
@ 2016-10-26 12:32   ` Imre Deak
  2016-10-26 16:13     ` Vivi, Rodrigo
  0 siblings, 1 reply; 6+ messages in thread
From: Imre Deak @ 2016-10-26 12:32 UTC (permalink / raw)
  To: Vivi, Rodrigo, Srivatsa, Anusha; +Cc: intel-gfx

On ti, 2016-10-25 at 18:00 +0000, Vivi, Rodrigo wrote:
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> On Mon, 2016-10-24 at 17:28 -0700, Anusha Srivatsa wrote:
> > Currently, for display there is only one DMC image for KBL.
> > Remove the stepping_info table for KBL and use the no_stepping_info
> > array for loading the firmware.
> > 
> > v2: Removed the block of code as pointed out by Rodrigo to make the
> > loads as generic as possible.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>

I assume the goal is to simplify the code. kbl_dmc_ver1_01.bin has
indeed only a single *,* entry and I assume it was confirmed that there
is no plan to release KBL firmware images in the future with multiple
stepping entries. With that this looks ok:
Acked-by: Imre Deak <imre.deak@intel.com>

I think it would be good to also emit a warn from the driver if we use
no_stepping_info and there are stepping entries in the firmware image
other than *,* since then we may end up loading the wrong firmware
version.

> > ---
> >  drivers/gpu/drm/i915/intel_csr.c | 11 +----------
> >  1 file changed, 1 insertion(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > b/drivers/gpu/drm/i915/intel_csr.c
> > index 1ea0e1f..d7a04bc 100644
> > --- a/drivers/gpu/drm/i915/intel_csr.c
> > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > @@ -168,12 +168,6 @@ struct stepping_info {
> >  	char substepping;
> >  };
> >  
> > -static const struct stepping_info kbl_stepping_info[] = {
> > -	{'A', '0'}, {'B', '0'}, {'C', '0'},
> > -	{'D', '0'}, {'E', '0'}, {'F', '0'},
> > -	{'G', '0'}, {'H', '0'}, {'I', '0'},
> > -};
> > -
> >  static const struct stepping_info skl_stepping_info[] = {
> >  	{'A', '0'}, {'B', '0'}, {'C', '0'},
> >  	{'D', '0'}, {'E', '0'}, {'F', '0'},
> > @@ -194,10 +188,7 @@ intel_get_stepping_info(struct
> > drm_i915_private *dev_priv)
> >  	const struct stepping_info *si;
> >  	unsigned int size;
> >  
> > -	if (IS_KABYLAKE(dev_priv)) {
> > -		size = ARRAY_SIZE(kbl_stepping_info);
> > -		si = kbl_stepping_info;
> > -	} else if (IS_SKYLAKE(dev_priv)) {
> > +	if (IS_SKYLAKE(dev_priv)) {
> >  		size = ARRAY_SIZE(skl_stepping_info);
> >  		si = skl_stepping_info;
> >  	} else if (IS_BROXTON(dev_priv)) {
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array
  2016-10-26 12:32   ` Imre Deak
@ 2016-10-26 16:13     ` Vivi, Rodrigo
  2016-10-26 21:23       ` Vivi, Rodrigo
  0 siblings, 1 reply; 6+ messages in thread
From: Vivi, Rodrigo @ 2016-10-26 16:13 UTC (permalink / raw)
  To: Deak, Imre; +Cc: intel-gfx

On Wed, 2016-10-26 at 15:32 +0300, Imre Deak wrote:
> On ti, 2016-10-25 at 18:00 +0000, Vivi, Rodrigo wrote:
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > On Mon, 2016-10-24 at 17:28 -0700, Anusha Srivatsa wrote:
> > > Currently, for display there is only one DMC image for KBL.
> > > Remove the stepping_info table for KBL and use the no_stepping_info
> > > array for loading the firmware.
> > > 
> > > v2: Removed the block of code as pointed out by Rodrigo to make the
> > > loads as generic as possible.
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> 
> I assume the goal is to simplify the code. kbl_dmc_ver1_01.bin has
> indeed only a single *,* entry and I assume it was confirmed that there
> is no plan to release KBL firmware images in the future with multiple
> stepping entries. With that this looks ok:

That is the case.

> Acked-by: Imre Deak <imre.deak@intel.com>

Thanks.

> 
> I think it would be good to also emit a warn from the driver if we use
> no_stepping_info and there are stepping entries in the firmware image
> other than *,* since then we may end up loading the wrong firmware
> version.

yeap, this seems a good idea. a conservative approach.
although, along with the confirmation we got the promise
that this kind of information would be now on in the official
release notes we receive along with the binary images.


> 
> > > ---
> > >  drivers/gpu/drm/i915/intel_csr.c | 11 +----------
> > >  1 file changed, 1 insertion(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > b/drivers/gpu/drm/i915/intel_csr.c
> > > index 1ea0e1f..d7a04bc 100644
> > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > @@ -168,12 +168,6 @@ struct stepping_info {
> > >  	char substepping;
> > >  };
> > >  
> > > -static const struct stepping_info kbl_stepping_info[] = {
> > > -	{'A', '0'}, {'B', '0'}, {'C', '0'},
> > > -	{'D', '0'}, {'E', '0'}, {'F', '0'},
> > > -	{'G', '0'}, {'H', '0'}, {'I', '0'},
> > > -};
> > > -
> > >  static const struct stepping_info skl_stepping_info[] = {
> > >  	{'A', '0'}, {'B', '0'}, {'C', '0'},
> > >  	{'D', '0'}, {'E', '0'}, {'F', '0'},
> > > @@ -194,10 +188,7 @@ intel_get_stepping_info(struct
> > > drm_i915_private *dev_priv)
> > >  	const struct stepping_info *si;
> > >  	unsigned int size;
> > >  
> > > -	if (IS_KABYLAKE(dev_priv)) {
> > > -		size = ARRAY_SIZE(kbl_stepping_info);
> > > -		si = kbl_stepping_info;
> > > -	} else if (IS_SKYLAKE(dev_priv)) {
> > > +	if (IS_SKYLAKE(dev_priv)) {
> > >  		size = ARRAY_SIZE(skl_stepping_info);
> > >  		si = skl_stepping_info;
> > >  	} else if (IS_BROXTON(dev_priv)) {
> > 
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array
  2016-10-26 16:13     ` Vivi, Rodrigo
@ 2016-10-26 21:23       ` Vivi, Rodrigo
  0 siblings, 0 replies; 6+ messages in thread
From: Vivi, Rodrigo @ 2016-10-26 21:23 UTC (permalink / raw)
  To: Deak, Imre; +Cc: intel-gfx


Patch merged to dinq. Thanks for the patch and thanks for the ack.

On Wed, 2016-10-26 at 09:12 -0700, Rodrigo Vivi wrote:
> On Wed, 2016-10-26 at 15:32 +0300, Imre Deak wrote:
> > On ti, 2016-10-25 at 18:00 +0000, Vivi, Rodrigo wrote:
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > 
> > > On Mon, 2016-10-24 at 17:28 -0700, Anusha Srivatsa wrote:
> > > > Currently, for display there is only one DMC image for KBL.
> > > > Remove the stepping_info table for KBL and use the no_stepping_info
> > > > array for loading the firmware.
> > > > 
> > > > v2: Removed the block of code as pointed out by Rodrigo to make the
> > > > loads as generic as possible.
> > > > 
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > 
> > I assume the goal is to simplify the code. kbl_dmc_ver1_01.bin has
> > indeed only a single *,* entry and I assume it was confirmed that there
> > is no plan to release KBL firmware images in the future with multiple
> > stepping entries. With that this looks ok:
> 
> That is the case.
> 
> > Acked-by: Imre Deak <imre.deak@intel.com>
> 
> Thanks.
> 
> > 
> > I think it would be good to also emit a warn from the driver if we use
> > no_stepping_info and there are stepping entries in the firmware image
> > other than *,* since then we may end up loading the wrong firmware
> > version.
> 
> yeap, this seems a good idea. a conservative approach.
> although, along with the confirmation we got the promise
> that this kind of information would be now on in the official
> release notes we receive along with the binary images.
> 
> 
> > 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_csr.c | 11 +----------
> > > >  1 file changed, 1 insertion(+), 10 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > > > b/drivers/gpu/drm/i915/intel_csr.c
> > > > index 1ea0e1f..d7a04bc 100644
> > > > --- a/drivers/gpu/drm/i915/intel_csr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > > > @@ -168,12 +168,6 @@ struct stepping_info {
> > > >  	char substepping;
> > > >  };
> > > >  
> > > > -static const struct stepping_info kbl_stepping_info[] = {
> > > > -	{'A', '0'}, {'B', '0'}, {'C', '0'},
> > > > -	{'D', '0'}, {'E', '0'}, {'F', '0'},
> > > > -	{'G', '0'}, {'H', '0'}, {'I', '0'},
> > > > -};
> > > > -
> > > >  static const struct stepping_info skl_stepping_info[] = {
> > > >  	{'A', '0'}, {'B', '0'}, {'C', '0'},
> > > >  	{'D', '0'}, {'E', '0'}, {'F', '0'},
> > > > @@ -194,10 +188,7 @@ intel_get_stepping_info(struct
> > > > drm_i915_private *dev_priv)
> > > >  	const struct stepping_info *si;
> > > >  	unsigned int size;
> > > >  
> > > > -	if (IS_KABYLAKE(dev_priv)) {
> > > > -		size = ARRAY_SIZE(kbl_stepping_info);
> > > > -		si = kbl_stepping_info;
> > > > -	} else if (IS_SKYLAKE(dev_priv)) {
> > > > +	if (IS_SKYLAKE(dev_priv)) {
> > > >  		size = ARRAY_SIZE(skl_stepping_info);
> > > >  		si = skl_stepping_info;
> > > >  	} else if (IS_BROXTON(dev_priv)) {
> > > 
> > > _______________________________________________
> > > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-10-26 21:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25  0:28 [PATCH] drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array Anusha Srivatsa
2016-10-25  0:46 ` ✗ Fi.CI.BAT: warning for " Patchwork
2016-10-25 18:00 ` [PATCH] " Vivi, Rodrigo
2016-10-26 12:32   ` Imre Deak
2016-10-26 16:13     ` Vivi, Rodrigo
2016-10-26 21:23       ` Vivi, Rodrigo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.