[Intel-gfx] [PATCH] [intel-gfx] drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL.

[Yadav, Jyoti R]

Hi Imre,

I also checked in Bspec. Good catch:)

FW size = (End address - Start Address) + 1

Will update the patch also. Thanks for quickly reviewing the patch.

Regards

Jyoti

_
On 8/29/2018 2:51 PM, Imre Deak wrote:
> On Tue, Aug 28, 2018 at 03:24:19PM -0400, Jyoti Yadav wrote:
>> From: Jyoti <jyoti.r.yadav at intel.com>
>>
>> This patch resolves the DMC FW loading issue.
>> Earlier DMC FW package have only one DMC FW for one stepping. But as such
>> there is no such restriction from Package side.
>> For ICL icl_dmc_ver1_07.bin binary package has DMC FW for 2 steppings.
>> So while reading the dmc_offset from package header, for 1st stepping offset
>> used to come 0x0 and was working fine till now.
>> But for second stepping and other steppings, offset is non zero numaber and is
>> in dwords. So we need to convert into bytes to fetch correct DMC FW from
>> correct place.
>>
>> v2 : Added check for DMC FW max size for various gen. (Imre Deak)
>> v3 : Corrected naming convention for various gen. (Imre Deak)
>> v4 : Initalized max_fw_size to 0
>>
>> Signed-off-by: Jyoti Yadav <jyoti.r.yadav at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_csr.c | 19 ++++++++++++++++---
>>   1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>> index 1ec4f09..d12051b 100644
>> --- a/drivers/gpu/drm/i915/intel_csr.c
>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>> @@ -55,7 +55,9 @@
>>   #define BXT_CSR_VERSION_REQUIRED	CSR_VERSION(1, 7)
>>   
>>   
>> -#define CSR_MAX_FW_SIZE			0x2FFF
>> +#define BXT_CSR_MAX_FW_SIZE		0x2FFF
>> +#define GLK_CSR_MAX_FW_SIZE		0x3FFF
>> +#define ICL_CSR_MAX_FW_SIZE		0x5FFF
> Err, just noticed that both the original and the new ones have
> an off-by-one error, could you still fix them while at it?
> (Correctly 0x3000, 0x4000, 0x6000 see 'Display CSR Program' in
> BSpec). With that and asssuming the description for the fw offset
> field will get clarified in BSpec:
>
> Reviewed-by: Imre Deak <imre.deak at intel.com>
>
>>   #define CSR_DEFAULT_FW_OFFSET		0xFFFFFFFF
>>   
>>   struct intel_css_header {
>> @@ -279,6 +281,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>>   	struct intel_csr *csr = &dev_priv->csr;
>>   	const struct stepping_info *si = intel_get_stepping_info(dev_priv);
>>   	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>> +	uint32_t max_fw_size = 0;
>>   	uint32_t i;
>>   	uint32_t *dmc_payload;
>>   	uint32_t required_version;
>> @@ -359,6 +362,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>>   			  si->stepping);
>>   		return NULL;
>>   	}
>> +	/* Convert dmc_offset into number of bytes. By default it is in dwords*/
>> +	dmc_offset *= 4;
>>   	readcount += dmc_offset;
>>   
>>   	/* Extract dmc_header information. */
>> @@ -391,8 +396,16 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>>   
>>   	/* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
>>   	nbytes = dmc_header->fw_size * 4;
>> -	if (nbytes > CSR_MAX_FW_SIZE) {
>> -		DRM_ERROR("DMC firmware too big (%u bytes)\n", nbytes);
>> +	if (INTEL_GEN(dev_priv) >= 11)
>> +		max_fw_size = ICL_CSR_MAX_FW_SIZE;
>> +	else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> +		max_fw_size = GLK_CSR_MAX_FW_SIZE;
>> +	else if (IS_GEN9(dev_priv))
>> +		max_fw_size = BXT_CSR_MAX_FW_SIZE;
>> +	else
>> +		MISSING_CASE(INTEL_REVID(dev_priv));
>> +	if (nbytes > max_fw_size) {
>> +		DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
>>   		return NULL;
>>   	}
>>   	csr->dmc_fw_size = dmc_header->fw_size;
>> -- 
>> 1.9.1
>>