| Message ID | 1535479881-5029-1-git-send-email-jyoti.r.yadav@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/intel_csr.c Fix DMC FW Loading issue on ICL. | expand |
Hi Jyoti,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.19-rc1 next-20180829]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jyoti-Yadav/drm-i915-intel_csr-c-Fix-DMC-FW-Loading-issue-on-ICL/20180829-212823
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x012-201834 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/gpu//drm/i915/intel_csr.c: In function 'csr_load_work_fn':
>> drivers/gpu//drm/i915/intel_csr.c:407:5: warning: 'max_fw_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
if (nbytes > max_fw_size) {
^
drivers/gpu//drm/i915/intel_csr.c:284:11: note: 'max_fw_size' was declared here
uint32_t max_fw_size;
^~~~~~~~~~~
vim +/max_fw_size +407 drivers/gpu//drm/i915/intel_csr.c
274
275 static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
276 const struct firmware *fw)
277 {
278 struct intel_css_header *css_header;
279 struct intel_package_header *package_header;
280 struct intel_dmc_header *dmc_header;
281 struct intel_csr *csr = &dev_priv->csr;
282 const struct stepping_info *si = intel_get_stepping_info(dev_priv);
283 uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
284 uint32_t max_fw_size;
285 uint32_t i;
286 uint32_t *dmc_payload;
287 uint32_t required_version;
288
289 if (!fw)
290 return NULL;
291
292 /* Extract CSS Header information*/
293 css_header = (struct intel_css_header *)fw->data;
294 if (sizeof(struct intel_css_header) !=
295 (css_header->header_len * 4)) {
296 DRM_ERROR("DMC firmware has wrong CSS header length "
297 "(%u bytes)\n",
298 (css_header->header_len * 4));
299 return NULL;
300 }
301
302 csr->version = css_header->version;
303
304 if (csr->fw_path == i915_modparams.dmc_firmware_path) {
305 /* Bypass version check for firmware override. */
306 required_version = csr->version;
307 } else if (IS_CANNONLAKE(dev_priv)) {
308 required_version = CNL_CSR_VERSION_REQUIRED;
309 } else if (IS_GEMINILAKE(dev_priv)) {
310 required_version = GLK_CSR_VERSION_REQUIRED;
311 } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
312 required_version = KBL_CSR_VERSION_REQUIRED;
313 } else if (IS_SKYLAKE(dev_priv)) {
314 required_version = SKL_CSR_VERSION_REQUIRED;
315 } else if (IS_BROXTON(dev_priv)) {
316 required_version = BXT_CSR_VERSION_REQUIRED;
317 } else {
318 MISSING_CASE(INTEL_REVID(dev_priv));
319 required_version = 0;
320 }
321
322 if (csr->version != required_version) {
323 DRM_INFO("Refusing to load DMC firmware v%u.%u,"
324 " please use v%u.%u\n",
325 CSR_VERSION_MAJOR(csr->version),
326 CSR_VERSION_MINOR(csr->version),
327 CSR_VERSION_MAJOR(required_version),
328 CSR_VERSION_MINOR(required_version));
329 return NULL;
330 }
331
332 readcount += sizeof(struct intel_css_header);
333
334 /* Extract Package Header information*/
335 package_header = (struct intel_package_header *)
336 &fw->data[readcount];
337 if (sizeof(struct intel_package_header) !=
338 (package_header->header_len * 4)) {
339 DRM_ERROR("DMC firmware has wrong package header length "
340 "(%u bytes)\n",
341 (package_header->header_len * 4));
342 return NULL;
343 }
344 readcount += sizeof(struct intel_package_header);
345
346 /* Search for dmc_offset to find firware binary. */
347 for (i = 0; i < package_header->num_entries; i++) {
348 if (package_header->fw_info[i].substepping == '*' &&
349 si->stepping == package_header->fw_info[i].stepping) {
350 dmc_offset = package_header->fw_info[i].offset;
351 break;
352 } else if (si->stepping == package_header->fw_info[i].stepping &&
353 si->substepping == package_header->fw_info[i].substepping) {
354 dmc_offset = package_header->fw_info[i].offset;
355 break;
356 } else if (package_header->fw_info[i].stepping == '*' &&
357 package_header->fw_info[i].substepping == '*')
358 dmc_offset = package_header->fw_info[i].offset;
359 }
360 if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
361 DRM_ERROR("DMC firmware not supported for %c stepping\n",
362 si->stepping);
363 return NULL;
364 }
365 /* Convert dmc_offset into number of bytes. By default it is in dwords*/
366 dmc_offset *= 4;
367 readcount += dmc_offset;
368
369 /* Extract dmc_header information. */
370 dmc_header = (struct intel_dmc_header *)&fw->data[readcount];
371 if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) {
372 DRM_ERROR("DMC firmware has wrong dmc header length "
373 "(%u bytes)\n",
374 (dmc_header->header_len));
375 return NULL;
376 }
377 readcount += sizeof(struct intel_dmc_header);
378
379 /* Cache the dmc header info. */
380 if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
381 DRM_ERROR("DMC firmware has wrong mmio count %u\n",
382 dmc_header->mmio_count);
383 return NULL;
384 }
385 csr->mmio_count = dmc_header->mmio_count;
386 for (i = 0; i < dmc_header->mmio_count; i++) {
387 if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
388 dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
389 DRM_ERROR("DMC firmware has wrong mmio address 0x%x\n",
390 dmc_header->mmioaddr[i]);
391 return NULL;
392 }
393 csr->mmioaddr[i] = _MMIO(dmc_header->mmioaddr[i]);
394 csr->mmiodata[i] = dmc_header->mmiodata[i];
395 }
396
397 /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
398 nbytes = dmc_header->fw_size * 4;
399 if (INTEL_GEN(dev_priv) >= 11)
400 max_fw_size = ICL_CSR_MAX_FW_SIZE;
401 else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
402 max_fw_size = GLK_CSR_MAX_FW_SIZE;
403 else if (IS_GEN9(dev_priv))
404 max_fw_size = BXT_CSR_MAX_FW_SIZE;
405 else
406 MISSING_CASE(INTEL_REVID(dev_priv));
> 407 if (nbytes > max_fw_size) {
408 DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
409 return NULL;
410 }
411 csr->dmc_fw_size = dmc_header->fw_size;
412
413 dmc_payload = kmalloc(nbytes, GFP_KERNEL);
414 if (!dmc_payload) {
415 DRM_ERROR("Memory allocation failed for dmc payload\n");
416 return NULL;
417 }
418
419 return memcpy(dmc_payload, &fw->data[readcount], nbytes);
420 }
421
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Jyoti,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.19-rc1 next-20180829]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jyoti-Yadav/drm-i915-intel_csr-c-Fix-DMC-FW-Loading-issue-on-ICL/20180829-212823
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x015-201834 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All errors (new ones prefixed by >>):
Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size
Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size
Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_read
Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32
Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD
Cyclomatic Complexity 4 include/linux/string.h:memcpy
Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order
Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_read
Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_read
Cyclomatic Complexity 5 arch/x86/include/asm/preempt.h:__preempt_count_add
Cyclomatic Complexity 1 arch/x86/include/asm/preempt.h:__preempt_count_dec_and_test
Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index
Cyclomatic Complexity 67 include/linux/slab.h:kmalloc_large
Cyclomatic Complexity 5 include/linux/slab.h:kmalloc
Cyclomatic Complexity 1 arch/x86/include/asm/io.h:writel
Cyclomatic Complexity 1 drivers/gpu//drm/i915/i915_reg.h:i915_mmio_reg_offset
Cyclomatic Complexity 1 drivers/gpu//drm/i915/i915_drv.h:intel_info
Cyclomatic Complexity 1 drivers/gpu//drm/i915/i915_drv.h:__raw_i915_write32
Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_csr.c:intel_get_stepping_info
Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_csr.c:gen9_set_dc_state_debugmask
Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_drv.h:assert_rpm_device_not_suspended
Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_drv.h:assert_rpm_wakelock_held
Cyclomatic Complexity 30 drivers/gpu//drm/i915/intel_csr.c:parse_csr_fw
Cyclomatic Complexity 1 include/linux/workqueue.h:queue_work
Cyclomatic Complexity 1 include/linux/workqueue.h:schedule_work
Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_csr.c:intel_csr_load_program
Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_csr.c:csr_load_work_fn
Cyclomatic Complexity 8 drivers/gpu//drm/i915/intel_csr.c:intel_csr_ucode_init
Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_csr.c:intel_csr_ucode_suspend
Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_csr.c:intel_csr_ucode_resume
Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_csr.c:intel_csr_ucode_fini
drivers/gpu//drm/i915/intel_csr.c: In function 'csr_load_work_fn':
>> drivers/gpu//drm/i915/intel_csr.c:407:5: error: 'max_fw_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (nbytes > max_fw_size) {
^
drivers/gpu//drm/i915/intel_csr.c:284:11: note: 'max_fw_size' was declared here
uint32_t max_fw_size;
^~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/max_fw_size +407 drivers/gpu//drm/i915/intel_csr.c
274
275 static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
276 const struct firmware *fw)
277 {
278 struct intel_css_header *css_header;
279 struct intel_package_header *package_header;
280 struct intel_dmc_header *dmc_header;
281 struct intel_csr *csr = &dev_priv->csr;
282 const struct stepping_info *si = intel_get_stepping_info(dev_priv);
283 uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
284 uint32_t max_fw_size;
285 uint32_t i;
286 uint32_t *dmc_payload;
287 uint32_t required_version;
288
289 if (!fw)
290 return NULL;
291
292 /* Extract CSS Header information*/
293 css_header = (struct intel_css_header *)fw->data;
294 if (sizeof(struct intel_css_header) !=
295 (css_header->header_len * 4)) {
296 DRM_ERROR("DMC firmware has wrong CSS header length "
297 "(%u bytes)\n",
298 (css_header->header_len * 4));
299 return NULL;
300 }
301
302 csr->version = css_header->version;
303
304 if (csr->fw_path == i915_modparams.dmc_firmware_path) {
305 /* Bypass version check for firmware override. */
306 required_version = csr->version;
307 } else if (IS_CANNONLAKE(dev_priv)) {
308 required_version = CNL_CSR_VERSION_REQUIRED;
309 } else if (IS_GEMINILAKE(dev_priv)) {
310 required_version = GLK_CSR_VERSION_REQUIRED;
311 } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
312 required_version = KBL_CSR_VERSION_REQUIRED;
313 } else if (IS_SKYLAKE(dev_priv)) {
314 required_version = SKL_CSR_VERSION_REQUIRED;
315 } else if (IS_BROXTON(dev_priv)) {
316 required_version = BXT_CSR_VERSION_REQUIRED;
317 } else {
318 MISSING_CASE(INTEL_REVID(dev_priv));
319 required_version = 0;
320 }
321
322 if (csr->version != required_version) {
323 DRM_INFO("Refusing to load DMC firmware v%u.%u,"
324 " please use v%u.%u\n",
325 CSR_VERSION_MAJOR(csr->version),
326 CSR_VERSION_MINOR(csr->version),
327 CSR_VERSION_MAJOR(required_version),
328 CSR_VERSION_MINOR(required_version));
329 return NULL;
330 }
331
332 readcount += sizeof(struct intel_css_header);
333
334 /* Extract Package Header information*/
335 package_header = (struct intel_package_header *)
336 &fw->data[readcount];
337 if (sizeof(struct intel_package_header) !=
338 (package_header->header_len * 4)) {
339 DRM_ERROR("DMC firmware has wrong package header length "
340 "(%u bytes)\n",
341 (package_header->header_len * 4));
342 return NULL;
343 }
344 readcount += sizeof(struct intel_package_header);
345
346 /* Search for dmc_offset to find firware binary. */
347 for (i = 0; i < package_header->num_entries; i++) {
348 if (package_header->fw_info[i].substepping == '*' &&
349 si->stepping == package_header->fw_info[i].stepping) {
350 dmc_offset = package_header->fw_info[i].offset;
351 break;
352 } else if (si->stepping == package_header->fw_info[i].stepping &&
353 si->substepping == package_header->fw_info[i].substepping) {
354 dmc_offset = package_header->fw_info[i].offset;
355 break;
356 } else if (package_header->fw_info[i].stepping == '*' &&
357 package_header->fw_info[i].substepping == '*')
358 dmc_offset = package_header->fw_info[i].offset;
359 }
360 if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
361 DRM_ERROR("DMC firmware not supported for %c stepping\n",
362 si->stepping);
363 return NULL;
364 }
365 /* Convert dmc_offset into number of bytes. By default it is in dwords*/
366 dmc_offset *= 4;
367 readcount += dmc_offset;
368
369 /* Extract dmc_header information. */
370 dmc_header = (struct intel_dmc_header *)&fw->data[readcount];
371 if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) {
372 DRM_ERROR("DMC firmware has wrong dmc header length "
373 "(%u bytes)\n",
374 (dmc_header->header_len));
375 return NULL;
376 }
377 readcount += sizeof(struct intel_dmc_header);
378
379 /* Cache the dmc header info. */
380 if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
381 DRM_ERROR("DMC firmware has wrong mmio count %u\n",
382 dmc_header->mmio_count);
383 return NULL;
384 }
385 csr->mmio_count = dmc_header->mmio_count;
386 for (i = 0; i < dmc_header->mmio_count; i++) {
387 if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
388 dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
389 DRM_ERROR("DMC firmware has wrong mmio address 0x%x\n",
390 dmc_header->mmioaddr[i]);
391 return NULL;
392 }
393 csr->mmioaddr[i] = _MMIO(dmc_header->mmioaddr[i]);
394 csr->mmiodata[i] = dmc_header->mmiodata[i];
395 }
396
397 /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
398 nbytes = dmc_header->fw_size * 4;
399 if (INTEL_GEN(dev_priv) >= 11)
400 max_fw_size = ICL_CSR_MAX_FW_SIZE;
401 else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
402 max_fw_size = GLK_CSR_MAX_FW_SIZE;
403 else if (IS_GEN9(dev_priv))
404 max_fw_size = BXT_CSR_MAX_FW_SIZE;
405 else
406 MISSING_CASE(INTEL_REVID(dev_priv));
> 407 if (nbytes > max_fw_size) {
408 DRM_ERROR("DMC FW too big (%u bytes)\n", nbytes);
409 return NULL;
410 }
411 csr->dmc_fw_size = dmc_header->fw_size;
412
413 dmc_payload = kmalloc(nbytes, GFP_KERNEL);
414 if (!dmc_payload) {
415 DRM_ERROR("Memory allocation failed for dmc payload\n");
416 return NULL;
417 }
418
419 return memcpy(dmc_payload, &fw->data[readcount], nbytes);
420 }
421
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 1ec4f09..58d8bda 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 #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; 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;