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-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
