[Intel-gfx] [PATCH] drm/i915/DMC/KBL: Load DMC on KBL using the no_stepping_info array

[Vivi, Rodrigo]

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 at 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 at intel.com>
> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa at 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 at 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 at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>