Re: [PATCH 3/3] drm/i915: refactor stepping info retrieval

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 21 Oct 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Oct 20, 2015 at 03:38:33PM +0300, Jani Nikula wrote:
>> Have only one if ladder for platforms and only one range check for
>> size. Makes it easier to handle new platforms. Remove the use of
>> negative return values in char, which might underflow to be positive for
>> some negative error codes.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> On the series: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Apparently this conflicts with some other firmware patches. I'll let
Imre decide if we should give them priority, or just merge these.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/intel_csr.c | 46 ++++++++++++++++++++--------------------
>>  1 file changed, 23 insertions(+), 23 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>> index fe9a55cf8212..3e7953254803 100644
>> --- a/drivers/gpu/drm/i915/intel_csr.c
>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>> @@ -188,28 +188,25 @@ static const struct stepping_info bxt_stepping_info[] = {
>>  	{'B', '0'}, {'B', '1'}, {'B', '2'}
>>  };
>>  
>> -static char intel_get_stepping(struct drm_device *dev)
>> +static const struct stepping_info *intel_get_stepping_info(struct drm_device *dev)
>>  {
>> -	if (IS_SKYLAKE(dev) && (dev->pdev->revision <
>> -			ARRAY_SIZE(skl_stepping_info)))
>> -		return skl_stepping_info[dev->pdev->revision].stepping;
>> -	else if (IS_BROXTON(dev) && (dev->pdev->revision <
>> -				ARRAY_SIZE(bxt_stepping_info)))
>> -		return bxt_stepping_info[dev->pdev->revision].stepping;
>> -	else
>> -		return -ENODATA;
>> -}
>> +	const struct stepping_info *si;
>> +	unsigned int size;
>> +
>> +	if (IS_SKYLAKE(dev)) {
>> +		size = ARRAY_SIZE(skl_stepping_info);
>> +		si = skl_stepping_info;
>> +	} else if (IS_BROXTON(dev)) {
>> +		size = ARRAY_SIZE(bxt_stepping_info);
>> +		si = bxt_stepping_info;
>> +	} else {
>> +		return NULL;
>> +	}
>>  
>> -static char intel_get_substepping(struct drm_device *dev)
>> -{
>> -	if (IS_SKYLAKE(dev) && (dev->pdev->revision <
>> -			ARRAY_SIZE(skl_stepping_info)))
>> -		return skl_stepping_info[dev->pdev->revision].substepping;
>> -	else if (IS_BROXTON(dev) && (dev->pdev->revision <
>> -			ARRAY_SIZE(bxt_stepping_info)))
>> -		return bxt_stepping_info[dev->pdev->revision].substepping;
>> -	else
>> -		return -ENODATA;
>> +	if (INTEL_REVID(dev) < size)
>> +		return si + INTEL_REVID(dev);
>> +
>> +	return NULL;
>>  }
>>  
>>  /**
>> @@ -296,8 +293,8 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>>  	struct intel_package_header *package_header;
>>  	struct intel_dmc_header *dmc_header;
>>  	struct intel_csr *csr = &dev_priv->csr;
>> -	char stepping = intel_get_stepping(dev);
>> -	char substepping = intel_get_substepping(dev);
>> +	const struct stepping_info *stepping_info = intel_get_stepping_info(dev);
>> +	char stepping, substepping;
>>  	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>>  	uint32_t i;
>>  	uint32_t *dmc_payload;
>> @@ -308,11 +305,14 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>>  		goto out;
>>  	}
>>  
>> -	if ((stepping == -ENODATA) || (substepping == -ENODATA)) {
>> +	if (!stepping_info) {
>>  		DRM_ERROR("Unknown stepping info, firmware loading failed\n");
>>  		goto out;
>>  	}
>>  
>> +	stepping = stepping_info->stepping;
>> +	substepping = stepping_info->substepping;
>> +
>>  	/* Extract CSS Header information*/
>>  	css_header = (struct intel_css_header *)fw->data;
>>  	if (sizeof(struct intel_css_header) !=
>> -- 
>> 2.1.4
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx