Il 01/10/2018 11:36, Dan Carpenter ha scritto:
Hello Giulio Benetti,
The patch 490cda5a3c82: "drm/sun4i: Handle DRM_BUS_FLAG_PIXDATA_*EDGE
checking if panel is used." from Jul 18, 2018, leads to the following
static checker warning:
drivers/gpu/drm/sun4i/sun4i_tcon.c:558 sun4i_tcon0_mode_set_rgb()
warn: 'tcon->panel' isn't an ERR_PTR
drivers/gpu/drm/sun4i/sun4i_tcon.c
481 const struct drm_display_mode
*mode)
482 {
483 unsigned int bp, hsync, vsync;
484 u8 clk_delay;
485 u32 val = 0;
486
487 WARN_ON(!tcon->quirks->has_channel_0);
488
489 tcon->dclk_min_div = 6;
490 tcon->dclk_max_div = 127;
491 sun4i_tcon0_mode_set_common(tcon, mode);
492
493 /* Set dithering if needed */
494 sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
^^^^^^^^^^^^^
Here there should be something like:
if (!tcon->panel) {
sun4i_tcon0_mode_set_dithering(tcon, tcon->panel->connector);
}
495
496 /* Adjust clock delay */
497 clk_delay = sun4i_tcon_get_clk_delay(mode, 0);
498 regmap_update_bits(tcon->regs, SUN4I_TCON0_CTL_REG,
499 SUN4I_TCON0_CTL_CLK_DELAY_MASK,
500 SUN4I_TCON0_CTL_CLK_DELAY(clk_delay));
501
502 /*
503 * This is called a backporch in the register documentation,
504 * but it really is the back porch + hsync
505 */
506 bp = mode->crtc_htotal - mode->crtc_hsync_start;
507 DRM_DEBUG_DRIVER("Setting horizontal total %d, backporch %d\n",
508 mode->crtc_htotal, bp);
509
510 /* Set horizontal display timings */
511 regmap_write(tcon->regs, SUN4I_TCON0_BASIC1_REG,
512 SUN4I_TCON0_BASIC1_H_TOTAL(mode->crtc_htotal) |
513 SUN4I_TCON0_BASIC1_H_BACKPORCH(bp));
514
515 /*
516 * This is called a backporch in the register documentation,
517 * but it really is the back porch + hsync
518 */
519 bp = mode->crtc_vtotal - mode->crtc_vsync_start;
520 DRM_DEBUG_DRIVER("Setting vertical total %d, backporch %d\n",
521 mode->crtc_vtotal, bp);
522
523 /* Set vertical display timings */
524 regmap_write(tcon->regs, SUN4I_TCON0_BASIC2_REG,
525 SUN4I_TCON0_BASIC2_V_TOTAL(mode->crtc_vtotal * 2)
|
526 SUN4I_TCON0_BASIC2_V_BACKPORCH(bp));
527
528 /* Set Hsync and Vsync length */
529 hsync = mode->crtc_hsync_end - mode->crtc_hsync_start;
530 vsync = mode->crtc_vsync_end - mode->crtc_vsync_start;
531 DRM_DEBUG_DRIVER("Setting HSYNC %d, VSYNC %d\n", hsync, vsync);
532 regmap_write(tcon->regs, SUN4I_TCON0_BASIC3_REG,
533 SUN4I_TCON0_BASIC3_V_SYNC(vsync) |
534 SUN4I_TCON0_BASIC3_H_SYNC(hsync));
535
536 /* Setup the polarity of the various signals */
537 if (mode->flags & DRM_MODE_FLAG_PHSYNC)
538 val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
539
540 if (mode->flags & DRM_MODE_FLAG_PVSYNC)
541 val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
542
543 /*
544 * On A20 and similar SoCs, the only way to achieve Positive
Edge
545 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
546 * By default TCON works in Negative Edge(Falling Edge),
547 * this is why phase is set to 0 in that case.
548 * Unfortunately there's no way to logically invert dclk
through
549 * IO_POL register.
550 * The only acceptable way to work, triple checked with scope,
551 * is using clock phase set to 0° for Negative Edge and set to
240°
552 * for Positive Edge.
553 * On A33 and similar SoCs there would be a 90° phase option,
554 * but it divides also dclk by 2.
555 * Following code is a way to avoid quirks all around TCON
556 * and DOTCLOCK drivers.
557 */
558 if (!IS_ERR(tcon->panel)) {
^^^^^^^^^^^^^^^^^^
Unpossible to be an error pointer!
So maybe I should use IS_ERR_OR_NULL() instead of IS_ERR(), or maybe simply:
if (!tcon->panel) {
Right?
Anyway I'm going to test it with "sparse".
Is it the same tool you've used for this?
Thanks
Kind regards
--
Giulio Benetti
CTO
MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
