[linux-sunxi] Re: [PATCH v9 1/5] drm/sun4i: sun6i_mipi_dsi: Fix hsync_porch overflow

2019-03-04 Thread Maxime Ripard 
On Sun, Mar 03, 2019 at 11:05:23PM +0530, Jagan Teki wrote:
> Loop N1 instruction delay for burst mode devices are computed
> based on horizontal sync and porch timing values.
> 
> The current driver is using u16 type for computing this hsync_porch
> value, which would failed to fit within the u16 type for large sync
> and porch timings devices. This would result in hsync_porch overflow
> and eventually computed wrong instruction delay value.
> 
> Example, timings, where it produces the overflow
> {
>   .hdisplay   = 1080,
>   .hsync_start= 1080 + 408,
> .hsync_end  = 1080 + 408 + 4,
> .htotal = 1080 + 408 + 4 + 38,
> }
> 
> It reproduces the desired delay value 65487 but the correct working
> value should be 7.
> 
> So, Fix it by computing hsync_porch value separately with u32 type.
> 
> Fixes: 1c1a7aa3663c ("drm/sun4i: dsi: Add burst support")
> Signed-off-by: Jagan Teki 
> Tested-by: Merlijn Wajer 
> ---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c 
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 6ff585055a07..465e7fc57899 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -457,8 +457,9 @@ static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi 
> *dsi,
>   u16 delay = 50 - 1;
>  
>   if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> - delay = (mode->htotal - mode->hdisplay) * 150;
> - delay /= (mode->clock / 1000) * 8;
> + u32 hsync_porch = (mode->htotal - mode->hdisplay);
> +
> + delay = ((hsync_porch * 150) / ((mode->clock / 1000) * 8));

shouldn't we keep the multiplication by 150 in the u32 assignation?
Otherwise, we could keep a u16 for the delay

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature

[linux-sunxi] Re: [PATCH v9 5/5] drm/sun4i: sun6i_mipi_dsi: Simplify dsi setup timings code

2019-03-04 Thread Maxime Ripard 
On Sun, Mar 03, 2019 at 11:05:27PM +0530, Jagan Teki wrote:
> DSI timings are varies between burst/non-burst devices and
> current driver is handling this support via if, else statements
> which would difficult to read.
> 
> Simplify it by using goto to make code more readable.
> 
> Signed-off-by: Jagan Teki 
> Tested-by: Merlijn Wajer 
> ---
> Note: This change is created based on previous version burst
> changes [1] by addressing comment from [2] by Maxime to make
> code readable. 
> 
> [1] https://patchwork.kernel.org/cover/10813623/
> [2] https://patchwork.kernel.org/patch/1007/
> 
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 79 ++
>  1 file changed, 42 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c 
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 31ec4048804d..898f32319c2d 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -550,7 +550,8 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>  {
>   struct mipi_dsi_device *device = dsi->device;
>   unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> - u16 hbp = 0, hfp = 0, hsa = 0, hblk = 0, vblk = 0;
> + u16 hbp, hfp, hsa, hblk;
> + u16 vblk = 0;
>   u32 basic_ctl = 0;
>   size_t bytes;
>   u8 *buffer;
> @@ -558,6 +559,7 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>   /* Do all timing calculations up front to allocate buffer space */
>  
>   if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> + hbp = hfp = hsa = 0;

This raises a checkpatch warning and isn't necessary

>   hblk = mode->hdisplay * Bpp;
>   basic_ctl = SUN6I_DSI_BASIC_CTL_VIDEO_BURST |
>   SUN6I_DSI_BASIC_CTL_HSA_HSE_DIS |
> @@ -566,48 +568,51 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi 
> *dsi,
>   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
> -  */
> +
> + goto alloc_buf;

And I'd rather not have a goto in the middle of the code for no
particular reason.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature

[linux-sunxi] Re: [PATCH v9 3/5] drm/sun4i: sun6i_mipi_dsi: Support vblk timing for 4-lane devices

2019-03-04 Thread Maxime Ripard 
On Sun, Mar 03, 2019 at 11:05:25PM +0530, Jagan Teki wrote:
> Like other dsi setup timings, or hblk for that matter vblk would
> also require compute the timings based payload equation along with
> packet overhead.
> 
> But, on the other hand vblk computation is also depends on device
> lane number.
> - for 4 lane devices, it is computed based on vtotal, packet overhead
>   along with hblk value.
> - for others devices, it is simply 0
> 
> BSP code from BPI-M64-bsp is computing vblk as for 4-lane devices
> (from linux-sunxi
> drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> 
> tmp = (ht*dsi_pixel_bits[format]/8)*vt-(4+dsi_hblk+2);
> dsi_vblk = (lane-tmp%lane);
> 
> So, update the vblk timing calculation to support all type of
> devices.
> 
> Tested on 2-lane, 4-lane MIPI-DSI LCD panels.

You should be explaining which issue you faced, in which setup, what
were its symptoms and how that solution is fixing it.

> Signed-off-by: Jagan Teki 
> Tested-by: Merlijn Wajer 
> ---
>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 27 +++---
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c 
> b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 140e55f5ed2e..b38358465d87 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -527,6 +527,24 @@ static void sun6i_dsi_setup_format(struct sun6i_dsi *dsi,
>SUN6I_DSI_PIXEL_CTL0_FORMAT(fmt));
>  }
>  
> +static u16 sun6i_dsi_get_timings_vblk(struct sun6i_dsi *dsi,
> +   struct drm_display_mode *mode, u16 hblk)
> +{
> + struct mipi_dsi_device *device = dsi->device;
> + unsigned int Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
> + int tmp;
> +
> + /*
> +  * The vertical blank is set using a blanking packet (4 bytes +
> +  * payload + 2 bytes). Its minimal size is therefore 6 bytes
> +  */
> +#define VBLK_PACKET_OVERHEAD 6
> + tmp = (mode->htotal * Bpp) * mode->vtotal -
> +   (hblk + VBLK_PACKET_OVERHEAD);
> +
> + return (device->lanes - tmp % device->lanes);

This should have a comment explaining why it's needed

> +}
> +
>  static void sun6i_dsi_setup_timings(struct sun6i_dsi *dsi,
>   struct drm_display_mode *mode)
>  {
> @@ -586,13 +604,8 @@ static void sun6i_dsi_setup_timings(struct sun6i_dsi 
> *dsi,
>  (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;
> + if (device->lanes == 4)

And that can be done in the function itself.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature

[linux-sunxi] Re: [PATCH v9 2/5] drm/sun4i: sun6i_mipi_dsi: Fix TCON DRQ set bits

2019-03-04 Thread Maxime Ripard 
On Sun, Mar 03, 2019 at 11:05:24PM +0530, Jagan Teki wrote:
> TCON DRQ for non-burst DSI mode can computed based on horizontal
> front porch value, but the current driver trying to include sync
> timings along with front porch resulting wrong drq.
> 
> This patch is trying to update the drq by subtracting hsync_start
> with hdisplay, which is horizontal front porch.
> 
> Current code:
> 
> mode->hsync_end - mode->hdisplay => horizontal front porch + sync
> 
> With this patch:
> 
> mode->hsync_start - mode->hdisplay => horizontal front porch
> 
> BSP code form BPI-M64-bsp is computing TCON DRQ set bits
> for non-burts as (from linux-sunxi/
> drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/de_dsi.c)
> 
> => panel->lcd_ht -panel->lcd_x - panel->lcd_hbp
> => (timmings->hor_front_porch + panel->lcd_hbp + panel->lcd_x)
  ^ + sync length +  
>- panel->lcd_x - panel->hbp
> => timmings->hor_front_porch
   ^ + sync
> => mode->hsync_start - mode->hdisplay

s/hsync_start/hsync_end/

Did you encounter any panel where this was fixing something? If so,
which one, and what is the matching timings and / or datasheet?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature

Re: [linux-sunxi] [RESEND PATCH] drm/sun4i: hdmi: Improve compatibility with hpd-less HDMI displays

2019-03-04 Thread Luc Verhaegen 
On Mon, Mar 04, 2019 at 03:06:16PM +0200, Priit Laes wrote:
> From: Priit Laes 
> 
> Even though HDMI connector features hotplug detect pin (HPD), there
> are older devices which do not support it. For these devices fall
> back to additional check on I2C bus to probe for EDID data.
> 
> One known example is HDMI/DVI display with following edid:
> 
> $ xxd -p display.edid
> 000005a1e0030100150f0103800f05780a0f6ea05748
> 9a2610474f20010101010101010101010101010101012a08804520e0
> 0b10200040009536001800fd0034441a2403000a202020202020
> 001000310a202020202020202020202000102a4030701300
> 782d111e006b
> 
> $ edid-decode display.edid
> EDID version: 1.3
> Manufacturer: AMA Model 3e0 Serial Number 1
> Digital display
> Maximum image size: 15 cm x 5 cm
> Gamma: 2.20
> RGB color display
> First detailed timing is preferred timing
> Display x,y Chromaticity:
>   Red:   0.6250, 0.3398
>   Green: 0.2841, 0.6044
>   Blue:  0.1494, 0.0644
>   White: 0.2802, 0.3105
> 
> Established timings supported:
>   640x480@60Hz 4:3 HorFreq: 31469 Hz Clock: 25.175 MHz
> Standard timings supported:
> Detailed mode: Clock 20.900 MHz, 149 mm x 54 mm
>640  672  672  709 hborder 0
>480  484  484  491 vborder 0
>-hsync -vsync
>VertFreq: 60 Hz, HorFreq: 29478 Hz
> Monitor ranges (GTF): 52-68Hz V, 26-36kHz H, max dotclock 30MHz
> Dummy block
> Dummy block
> Checksum: 0x6b (valid)
> 
> Now, current implementation is still flawed, as HDMI uses the
> HPD signal to indicate that the source should re-read the EDID
> due to change in device capabilities. With current HPD polling
> implementation we would most certainly miss those notifications
> as one can try just swapping two HDMI monitors really fast.
> 
> Proper fix would be skipping the HPD pin detection and relying
> on just EDID fetching and acting on its changes.

HPD has been a hard requirement since DDWG came up with DVI somewhere in 
the late 90s. This monitor is plainly broken, and should not get an 
expensive i2c address polling based workaround at the driver level.

Luc Verhaegen.

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] [RESEND PATCH] drm/sun4i: hdmi: Improve compatibility with hpd-less HDMI displays

2019-03-04 Thread Priit Laes 
From: Priit Laes 

Even though HDMI connector features hotplug detect pin (HPD), there
are older devices which do not support it. For these devices fall
back to additional check on I2C bus to probe for EDID data.

One known example is HDMI/DVI display with following edid:

$ xxd -p display.edid
000005a1e0030100150f0103800f05780a0f6ea05748
9a2610474f20010101010101010101010101010101012a08804520e0
0b10200040009536001800fd0034441a2403000a202020202020
001000310a202020202020202020202000102a4030701300
782d111e006b

$ edid-decode display.edid
EDID version: 1.3
Manufacturer: AMA Model 3e0 Serial Number 1
Digital display
Maximum image size: 15 cm x 5 cm
Gamma: 2.20
RGB color display
First detailed timing is preferred timing
Display x,y Chromaticity:
  Red:   0.6250, 0.3398
  Green: 0.2841, 0.6044
  Blue:  0.1494, 0.0644
  White: 0.2802, 0.3105

Established timings supported:
  640x480@60Hz 4:3 HorFreq: 31469 Hz Clock: 25.175 MHz
Standard timings supported:
Detailed mode: Clock 20.900 MHz, 149 mm x 54 mm
   640  672  672  709 hborder 0
   480  484  484  491 vborder 0
   -hsync -vsync
   VertFreq: 60 Hz, HorFreq: 29478 Hz
Monitor ranges (GTF): 52-68Hz V, 26-36kHz H, max dotclock 30MHz
Dummy block
Dummy block
Checksum: 0x6b (valid)

Now, current implementation is still flawed, as HDMI uses the
HPD signal to indicate that the source should re-read the EDID
due to change in device capabilities. With current HPD polling
implementation we would most certainly miss those notifications
as one can try just swapping two HDMI monitors really fast.

Proper fix would be skipping the HPD pin detection and relying
on just EDID fetching and acting on its changes.

CC: Russell King 
Signed-off-by: Priit Laes 
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c 
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 416da5376701..44d5d50b7ada 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -242,14 +242,18 @@ sun4i_hdmi_connector_detect(struct drm_connector 
*connector, bool force)
struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
unsigned long reg;
 
-   if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,
-  reg & SUN4I_HDMI_HPD_HIGH,
-  0, 50)) {
-   cec_phys_addr_invalidate(hdmi->cec_adap);
-   return connector_status_disconnected;
-   }
-
-   return connector_status_connected;
+   /* TODO: Drop HPD polling and instead keep track of EDID changes */
+   if (!readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,
+   reg & SUN4I_HDMI_HPD_HIGH,
+   0, 50))
+   return connector_status_connected;
+
+   /* Fall back to EDID in case display does not support HPD */
+   if (!IS_ERR(hdmi->i2c) && drm_probe_ddc(hdmi->i2c))
+   return connector_status_connected;
+
+   cec_phys_addr_invalidate(hdmi->cec_adap);
+   return connector_status_disconnected;
 }
 
 static const struct drm_connector_funcs sun4i_hdmi_connector_funcs = {
-- 
2.11.0

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.