On Mon, Jul 02, 2018 at 04:04:45PM +0300, Ville Syrjälä wrote: > On Sat, Jun 30, 2018 at 10:12:21PM +0300, Dan Carpenter wrote: > > Hi Ville, > > > > Thank you for the patch! Perhaps something to improve: > > > > url: https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-Third-attempt-at-fixing-the-fb-helper-best_encoder-mess/20180629-014202 > > base: git://people.freedesktop.org/~airlied/linux.git drm-next > > > > smatch warnings: > > drivers/gpu/drm/nouveau/nouveau_connector.c:461 nouveau_connector_ddc_detect() error: uninitialized symbol 'nv_encoder'. > > > > # https://github.com/0day-ci/linux/commit/7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06 > > git remote add linux-review https://github.com/0day-ci/linux > > git remote update linux-review > > git checkout 7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06 > > vim +/nv_encoder +461 drivers/gpu/drm/nouveau/nouveau_connector.c > > > > 6ee738610 Ben Skeggs 2009-12-11 407 > > 8777c5c11 Ben Skeggs 2014-06-06 408 static struct nouveau_encoder * > > 8777c5c11 Ben Skeggs 2014-06-06 409 nouveau_connector_ddc_detect(struct drm_connector *connector) > > 6ee738610 Ben Skeggs 2009-12-11 410 { > > 6ee738610 Ben Skeggs 2009-12-11 411 struct drm_device *dev = connector->dev; > > 1a1841d30 Ben Skeggs 2012-12-10 412 struct nouveau_connector *nv_connector = nouveau_connector(connector); > > 77145f1cb Ben Skeggs 2012-07-31 413 struct nouveau_drm *drm = nouveau_drm(dev); > > 1167c6bc5 Ben Skeggs 2016-05-18 414 struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device); > > 8777c5c11 Ben Skeggs 2014-06-06 415 struct nouveau_encoder *nv_encoder; > > 6d385c0aa Rob Clark 2014-07-17 416 struct drm_encoder *encoder; > > 1a1841d30 Ben Skeggs 2012-12-10 417 int i, panel = -ENODEV; > > 1a1841d30 Ben Skeggs 2012-12-10 418 > > 1a1841d30 Ben Skeggs 2012-12-10 419 /* eDP panels need powering on by us (if the VBIOS doesn't default it > > 1a1841d30 Ben Skeggs 2012-12-10 420 * to on) before doing any AUX channel transactions. LVDS panel power > > 1a1841d30 Ben Skeggs 2012-12-10 421 * is handled by the SOR itself, and not required for LVDS DDC. > > 1a1841d30 Ben Skeggs 2012-12-10 422 */ > > 1a1841d30 Ben Skeggs 2012-12-10 423 if (nv_connector->type == DCB_CONNECTOR_eDP) { > > 2ea7249fe Ben Skeggs 2015-08-20 424 panel = nvkm_gpio_get(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff); > > 1a1841d30 Ben Skeggs 2012-12-10 425 if (panel == 0) { > > 2ea7249fe Ben Skeggs 2015-08-20 426 nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, 1); > > 1a1841d30 Ben Skeggs 2012-12-10 427 msleep(300); > > 1a1841d30 Ben Skeggs 2012-12-10 428 } > > 1a1841d30 Ben Skeggs 2012-12-10 429 } > > 6ee738610 Ben Skeggs 2009-12-11 430 > > 7ec8bb653 Ville Syrjälä 2018-06-28 431 drm_connector_for_each_possible_encoder(connector, encoder, i) { > > 6d385c0aa Rob Clark 2014-07-17 432 nv_encoder = nouveau_encoder(encoder); > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > If we enter the loop that means nv_encoder is non-NULL. Smatch can't > > prove that we always enter the loop in this case for whatever reason but > > my guess is that we always do. > > > > 4ca2b7120 Francisco Jerez 2010-08-08 433 > > 8777c5c11 Ben Skeggs 2014-06-06 434 if (nv_encoder->dcb->type == DCB_OUTPUT_DP) { > > 8777c5c11 Ben Skeggs 2014-06-06 435 int ret = nouveau_dp_detect(nv_encoder); > > 52aa30f25 Ben Skeggs 2016-11-04 436 if (ret == NOUVEAU_DP_MST) > > 52aa30f25 Ben Skeggs 2016-11-04 437 return NULL; > > 52aa30f25 Ben Skeggs 2016-11-04 438 if (ret == NOUVEAU_DP_SST) > > 8777c5c11 Ben Skeggs 2014-06-06 439 break; > > 8777c5c11 Ben Skeggs 2014-06-06 440 } else > > 39c1c9011 Lukas Wunner 2016-01-11 441 if ((vga_switcheroo_handler_flags() & > > 39c1c9011 Lukas Wunner 2016-01-11 442 VGA_SWITCHEROO_CAN_SWITCH_DDC) && > > 39c1c9011 Lukas Wunner 2016-01-11 443 nv_encoder->dcb->type == DCB_OUTPUT_LVDS && > > 39c1c9011 Lukas Wunner 2016-01-11 444 nv_encoder->i2c) { > > 39c1c9011 Lukas Wunner 2016-01-11 445 int ret; > > 39c1c9011 Lukas Wunner 2016-01-11 446 vga_switcheroo_lock_ddc(dev->pdev); > > 39c1c9011 Lukas Wunner 2016-01-11 447 ret = nvkm_probe_i2c(nv_encoder->i2c, 0x50); > > 39c1c9011 Lukas Wunner 2016-01-11 448 vga_switcheroo_unlock_ddc(dev->pdev); > > 39c1c9011 Lukas Wunner 2016-01-11 449 if (ret) > > 39c1c9011 Lukas Wunner 2016-01-11 450 break; > > 39c1c9011 Lukas Wunner 2016-01-11 451 } else > > 8777c5c11 Ben Skeggs 2014-06-06 452 if (nv_encoder->i2c) { > > 2aa5eac51 Ben Skeggs 2015-08-20 453 if (nvkm_probe_i2c(nv_encoder->i2c, 0x50)) > > 1a1841d30 Ben Skeggs 2012-12-10 454 break; > > 6ee738610 Ben Skeggs 2009-12-11 455 } > > 6ee738610 Ben Skeggs 2009-12-11 456 } > > 6ee738610 Ben Skeggs 2009-12-11 457 > > 1a1841d30 Ben Skeggs 2012-12-10 458 /* eDP panel not detected, restore panel power GPIO to previous > > 1a1841d30 Ben Skeggs 2012-12-10 459 * state to avoid confusing the SOR for other output types. > > 1a1841d30 Ben Skeggs 2012-12-10 460 */ > > 8777c5c11 Ben Skeggs 2014-06-06 @461 if (!nv_encoder && panel == 0) > > ^^^^^^^^^^^ > > So testing for NULL doesn't make sense. It's either non-NULL if we > > entered the loop at least once or it's uninitialized if we didn't enter > > the loop. > > Yeah, seems like a bogus check. I suppose in theory you could have a > connector with no encoders, although I can't think of any good reason > why anyone would do that. In fact this thing seems to upset gcc as well. It's not particularly helpful in its warning message though: ../drivers/gpu/drm/nouveau/nouveau_connector.c: In function ‘nouveau_connector_detect’: ../drivers/gpu/drm/nouveau/nouveau_connector.c:597:33: warning: ‘nv_encoder’ may be used uninitialized in this function [-Wmaybe-uninitialized] if (nv_partner && ((nv_encoder->dcb->type == DCB_OUTPUT_ANALOG && ~~~~~~~~~~^~~~~ No indication that it's nouveau_connector_ddc_detect() that supposedly supplies the uninitialized pointer. I think I'll just toss in a =NULL in there to silence this noise. > > > > > 2ea7249fe Ben Skeggs 2015-08-20 462 nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, panel); > > 1a1841d30 Ben Skeggs 2012-12-10 463 > > 8777c5c11 Ben Skeggs 2014-06-06 464 return nv_encoder; > > 6ee738610 Ben Skeggs 2009-12-11 465 } > > 6ee738610 Ben Skeggs 2009-12-11 466 > > > > > > --- > > 0-DAY kernel test infrastructure Open Source Technology Center > > https://lists.01.org/pipermail/kbuild-all Intel Corporation > > -- > Ville Syrjälä > Intel > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx