Re: [bug report] drm/sun4i: Handle DRM_BUS_FLAG_PIXDATA_*EDGE checking if panel is used.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux