From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752273AbdECIlv (ORCPT ); Wed, 3 May 2017 04:41:51 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:37729 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751533AbdECIlk (ORCPT ); Wed, 3 May 2017 04:41:40 -0400 Date: Wed, 3 May 2017 10:41:38 +0200 From: Maxime Ripard To: Chen-Yu Tsai Cc: Mike Turquette , Stephen Boyd , dri-devel , Daniel Vetter , David Airlie , Mark Rutland , Rob Herring , devicetree , linux-clk , linux-arm-kernel , linux-kernel , linux-sunxi Subject: Re: [linux-sunxi] [PATCH 13/15] drm/sun4i: Add HDMI support Message-ID: <20170503084138.rye2i6qd4ghxifep@lukather> References: <20170426065005.zoewz53q7l7r5e7p@lukather> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="qq3kb2nbfsjelwzs" Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --qq3kb2nbfsjelwzs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 26, 2017 at 03:59:28PM +0800, Chen-Yu Tsai wrote: > >> > + writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) | > >> > + SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay), > >> > + hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG); > >> > + > >> > + x =3D mode->htotal - mode->hsync_start; > >> > + y =3D mode->vtotal - mode->vsync_start; > >> > >> I'm a bit skeptical about this one. All the other parameters are not > >> inclusive of other, why would this one be different? Shouldn't it > >> be "Xtotal - Xsync_end" instead? > > > > By the usual meaning of backporch, you're right. However, Allwinner's > > seems to have it's own, which is actually the backporch + sync length. > > > > We also have that on all the other connectors (and TCON), and this was > > confirmed at the time using a scope on an RGB signal. >=20 > Yes. On the later SoCs such as the A31, the user manual actually has > timing diagrams showing this. >=20 > Unlike the TCON, the HDMI controller's timings lists the front porch > separately, instead of an all inclusive Xtotal. This is what made me > look twice. This should be easy to confirm though. Since the HDMI modes > are well known and can be exactly reproduced on our hardware, we can > just check for any distortions or refresh rate errors. This isn't as trivial as that. Screens usually have some tolerancies on the timings, which will probably make it go unnoticed, even though they are wrong. > >> > >> > + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(= y), > >> > + hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG); > >> > + > >> > + x =3D mode->hsync_start - mode->hdisplay; > >> > + y =3D mode->vsync_start - mode->vdisplay; > >> > + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(= y), > >> > + hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG); > >> > + > >> > + x =3D mode->hsync_end - mode->hsync_start; > >> > + y =3D mode->vsync_end - mode->vsync_start; > >> > + writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(= y), > >> > + hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG); > >> > + > >> > + val =3D SUN4I_HDMI_VID_TIMING_POL_TX_CLK; > >> > + if (mode->flags & DRM_MODE_FLAG_PHSYNC) > >> > + val |=3D SUN4I_HDMI_VID_TIMING_POL_HSYNC; > >> > + > >> > + if (mode->flags & DRM_MODE_FLAG_PVSYNC) > >> > + val |=3D SUN4I_HDMI_VID_TIMING_POL_VSYNC; > >> > + > >> > + writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG); > >> > >> You don't handle the interlaced video here, even though you set > >> > >> hdmi->connector.interlace_allowed =3D true > >> > >> later. > > > > I'll fix that. > > > >> The double clock and double scan flags aren't handled either, though > >> I don't understand which one is supposed to represent the need for the > >> HDMI pixel repeater. AFAIK this is required for resolutions with pixel > >> clocks lower than 25 MHz, the lower limit of HDMI's TMDS link. > > > > I'm not sure about this one though. I'd like to keep things quite > > simple for now and build up on that once the basis is working. Is it > > common in the wild? >=20 > If you drive the display at SDTV resolutions, then yes. Mode lines from > my HDMI monitor: >=20 > 720x576i 50 720 732 795 864 576 580 586 625 flags: nhsync, nvsync, > interlace, dblclk; type: driver > 720x480i 60 720 739 801 858 480 488 494 525 flags: nhsync, nvsync, > interlace, dblclk; type: driver > 720x480i 60 720 739 801 858 480 488 494 525 flags: nhsync, nvsync, > interlace, dblclk; type: driver >=20 > AFAIK these are standard modes that all devices should support. Whether > they are used daily is another thing. Maybe block modes with dblclk > in .mode_fixup for now? That would rather be atomic_check and / or mode_valid, but yeah, I can do that. Thanks! Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --qq3kb2nbfsjelwzs Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAABCAAGBQJZCZe+AAoJEBx+YmzsjxAg7J4P/AzerOTgQEza2dvpr39ZRIbc GjqqM2WUWZQ/4/wE2eeZa7dOQWlhHryTODrtSts0mzw3dlEkmHUTa1tmjlmDRMYI orHoZRxNNUalfMxNsi+SCx1t/PHSiXtwQpKsGKZKlQwJfD2EuWVTzQzKETjX6+Ap fPoYB2ezCGpQ7aKmNJ5Z2W0YRgBZDPkZeBrgE/sf/7GAfp5icLBPFZyZOFICj/mV wdvtf/K7Jla1UiltD8PnsvlNL4cKdb7m8obftjvjTujun+YzFIf2H5ZYLFuwwN/8 kInYyYo60Mt8d7/WkZk3KPrs0E06biQeUfoKdU9sKxixg5d9hZEXjcPcOZPxCvtp 7pJRir0ID+Nprzx4KhKTOiDWKHUdO4s66ZseRhQSLhLFyNx4qC8qAxcIYm6RIf5t SF/BsTshnGmeQYtdvrh6hPncnHTnw7UQleScT6eOlbMnoIi/YhzsBFfrs7Q36IWf a0AhAF/uDThf9Q17/+jhk8v5RvZ1fZ3rbtupkvQZ8z96Pflvakaw3WodY0BXemAg 4UWWFGdOz9oycVl6FbTc1gfdtxIa+CLuQkATTffftR7qUsRCLhtVJt/KOHjFhhPQ SEpM8oFfZfZlfn+Iyjm0sj3FYcy+4kIe89IGZ7yBmi4DFQigTC1GEx3afn3Br/7F CS1apvaHUnp90C7fj1yY =SDnL -----END PGP SIGNATURE----- --qq3kb2nbfsjelwzs--