Dne petek, 12. avgust 2022 ob 05:16:23 CEST je Samuel Holland napisal(a):
> Currently, the packet overhead is subtracted using unsigned arithmetic.
> With a short sync pulse, this could underflow and wrap around to near
> the maximal u16 value. Fix this by using signed subtraction. The call to
> max() will correctly handle any negative numbers that are produced.
>
> Apply the same fix to the other timings, even though those subtractions
> are less likely to underflow.
>
> Fixes: 133add5b5ad4 ("drm/sun4i: Add Allwinner A31 MIPI-DSI controller
> support") Signed-off-by: Samuel Holland <sam...@sholland.org>
> ---
>
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c index b4dfa166eccd..34234a144e87
> 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -522,77 +522,77 @@ static void sun6i_dsi_setup_format(struct sun6i_dsi
> *dsi, SUN6I_DSI_PIXEL_PF1_CRC_INIT_LINE0(0xffff) |
> SUN6I_DSI_PIXEL_PF1_CRC_INIT_LINEN(0xffff));
>
> regmap_write(dsi->regs, SUN6I_DSI_PIXEL_CTL0_REG,
> SUN6I_DSI_PIXEL_CTL0_PD_PLUG_DISABLE |
> SUN6I_DSI_PIXEL_CTL0_FORMAT(fmt));
> }
>
> static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
> struct drm_display_mode
*mode)
> {
> struct mipi_dsi_device *device = dsi->device;
> - unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) /
8;
> + int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
Nit: mipi_dsi_pixel_format_to_bpp() can return -EINVAL in case of unsupported
format. Would it make sense to check it?
In any case:
Reviewed-by: Jernej Skrabec <jernej.skra...@gmail.com>
Best regards,
Jernej
> u16 hbp = 0, hfp = 0, hsa = 0, hblk = 0, vblk = 0;
> u32 basic_ctl = 0;
> size_t bytes;
> u8 *buffer;
>
> /* Do all timing calculations up front to allocate buffer space */
>
> if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> hblk = mode->hdisplay * Bpp;
> basic_ctl = SUN6I_DSI_BASIC_CTL_VIDEO_BURST |
> SUN6I_DSI_BASIC_CTL_HSA_HSE_DIS |
> SUN6I_DSI_BASIC_CTL_HBP_DIS;
>
> if (device->lanes == 4)
> basic_ctl |= SUN6I_DSI_BASIC_CTL_TRAIL_FILL
|
>
SUN6I_DSI_BASIC_CTL_TRAIL_INV(0xc);
> } else {
> /*
> * A sync period is composed of a blanking packet (4
> * bytes + payload + 2 bytes) and a sync event packet
> * (4 bytes). Its minimal size is therefore 10 bytes
> */
> #define HSA_PACKET_OVERHEAD 10
> - hsa = max((unsigned int)HSA_PACKET_OVERHEAD,
> + hsa = max(HSA_PACKET_OVERHEAD,
> (mode->hsync_end - mode->hsync_start) *
Bpp - HSA_PACKET_OVERHEAD);
>
> /*
> * The backporch is set using a blanking packet (4
> * bytes + payload + 2 bytes). Its minimal size is
> * therefore 6 bytes
> */
> #define HBP_PACKET_OVERHEAD 6
> - hbp = max((unsigned int)HBP_PACKET_OVERHEAD,
> + hbp = max(HBP_PACKET_OVERHEAD,
> (mode->htotal - mode->hsync_end) * Bpp -
HBP_PACKET_OVERHEAD);
>
> /*
> * The frontporch is set using a sync event (4 bytes)
> * and two blanking packets (each one is 4 bytes +
> * payload + 2 bytes). Its minimal size is therefore
> * 16 bytes
> */
> #define HFP_PACKET_OVERHEAD 16
> - hfp = max((unsigned int)HFP_PACKET_OVERHEAD,
> + hfp = max(HFP_PACKET_OVERHEAD,
> (mode->hsync_start - mode->hdisplay) * Bpp
- HFP_PACKET_OVERHEAD);
>
> /*
> * The blanking is set using a sync event (4 bytes)
> * and a blanking packet (4 bytes + payload + 2
> * bytes). Its minimal size is therefore 10 bytes.
> */
> #define HBLK_PACKET_OVERHEAD 10
> - hblk = max((unsigned int)HBLK_PACKET_OVERHEAD,
> + hblk = max(HBLK_PACKET_OVERHEAD,
> (mode->htotal - (mode->hsync_end - mode-
>hsync_start)) * Bpp -
> HBLK_PACKET_OVERHEAD);
>
> /*
> * And I'm not entirely sure what vblk is about. The
driver in
> * Allwinner BSP is using a rather convoluted
calculation
> * there only for 4 lanes. However, using 0 (the !4
lanes
> * case) even with a 4 lanes screen seems to work...
> */
> vblk = 0;
> }
