From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH 2/2] drm/vc4: Add support for interlaced modes on HDMI. Date: Wed, 28 Sep 2016 19:53:16 -0700 Message-ID: <87vaxfe7s3.fsf@eliezer.anholt.net> References: <20160929022045.24813-1-eric@anholt.net> <20160929022045.24813-2-eric@anholt.net> <57EC7E14.5050602@rock-chips.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0422294287==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id BC3C26E08E for ; Thu, 29 Sep 2016 02:53:18 +0000 (UTC) In-Reply-To: <57EC7E14.5050602@rock-chips.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Mark yao , dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org --===============0422294287== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mark yao writes: > On 2016=E5=B9=B409=E6=9C=8829=E6=97=A5 10:20, Eric Anholt wrote: >> We just needed to initialize a few more fields. >> >> Signed-off-by: Eric Anholt >> --- >> drivers/gpu/drm/vc4/vc4_crtc.c | 17 ++++++++++++++--- >> drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++++---- >> drivers/gpu/drm/vc4/vc4_regs.h | 3 +++ >> 3 files changed, 25 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_cr= tc.c >> index 8fc2b731b59a..d575f8aa3273 100644 >> --- a/drivers/gpu/drm/vc4/vc4_crtc.c >> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c >> @@ -428,13 +428,24 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc= *crtc) >> VC4_SET_FIELD(mode->vsync_start - mode->vdisplay, >> PV_VERTB_VFP) | >> VC4_SET_FIELD(vactive, PV_VERTB_VACTIVE)); >> + >> + /* We set up first field even mode for HDMI. VEC's >> + * NTSC mode would want first field odd instead, once >> + * we support it (to do so, set ODD_FIRST and put the >> + * delay in VSYNCD_EVEN instead). >> + */ >> + CRTC_WRITE(PV_V_CONTROL, >> + PV_VCONTROL_CONTINUOUS | >> + PV_VCONTROL_INTERLACE | >> + VC4_SET_FIELD(mode->htotal / 2, >> + PV_VCONTROL_ODD_DELAY)); >> + CRTC_WRITE(PV_VSYNCD_EVEN, 0); >> + } else { >> + CRTC_WRITE(PV_V_CONTROL, PV_VCONTROL_CONTINUOUS); >> } >>=20=20=20 >> CRTC_WRITE(PV_HACT_ACT, mode->hdisplay); >>=20=20=20 >> - CRTC_WRITE(PV_V_CONTROL, >> - PV_VCONTROL_CONTINUOUS | >> - (interlace ? PV_VCONTROL_INTERLACE : 0)); >>=20=20=20 >> CRTC_WRITE(PV_CONTROL, >> VC4_SET_FIELD(format, PV_CONTROL_FORMAT) | >> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hd= mi.c >> index 5770d6704f4b..6095e48fcf46 100644 >> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c >> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c >> @@ -246,7 +246,7 @@ static struct drm_connector *vc4_hdmi_connector_init= (struct drm_device *dev, >> connector->polled =3D (DRM_CONNECTOR_POLL_CONNECT | >> DRM_CONNECTOR_POLL_DISCONNECT); >>=20=20=20 >> - connector->interlace_allowed =3D 0; >> + connector->interlace_allowed =3D true; >> connector->doublescan_allowed =3D 0; >>=20=20=20 >> drm_mode_connector_attach_encoder(connector, encoder); >> @@ -278,8 +278,8 @@ static void vc4_hdmi_encoder_mode_set(struct drm_enc= oder *encoder, >> bool debug_dump_regs =3D false; >> bool hsync_pos =3D mode->flags & DRM_MODE_FLAG_PHSYNC; >> bool vsync_pos =3D mode->flags & DRM_MODE_FLAG_PVSYNC; >> - u32 vactive =3D (mode->vdisplay >> >> - ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0)); >> + bool interlaced =3D mode->flags & DRM_MODE_FLAG_INTERLACE; >> + u32 vactive =3D mode->vdisplay >> interlaced; > > How about use mode->crtc_vdisplay: > > see this: > drm_mode_set_crtcinfo() > > if (p->flags & DRM_MODE_FLAG_INTERLACE) { > if (adjust_flags & CRTC_INTERLACE_HALVE_V) { > p->crtc_vdisplay /=3D 2; > p->crtc_vsync_start /=3D 2; > p->crtc_vsync_end /=3D 2; > p->crtc_vtotal /=3D 2; > } > } That would require setting up the adjust_flags, and I thought in the DRM we were trying to move away from using it. Also it would be pretty strange to use just this one field from crtc_*. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIbBAEBCgAGBQJX7IIcAAoJELXWKTbR/J7odhQP9jtY49wE4+EUqL9fu8R1PwW8 XUAIHg51ahXEiU+2RTOBZsUO2TZvikdf9JNhMEcusEHjzWtbLDeiArQ2SdRMpUUY So0LvusNi0i1GwUL8/zd6JLjlzfz+A/V8JzHAgHzHHOdiGc7lKDUSi4ER86G0AL8 r3o0YnpQFClHExEytCG5Fpsg755UMm3+U3NWZtsWt6QyEoi4iXG4TNT+OzN/gg1e SfBmz4JRfdHGC9DcR/fQZ1zBeWEKgF5LNJ1RbFrZaCMn7IK/zjk4NUP535n/uhDa sL/wvsEGSZ56rBC/iyZMfEuqGp6Uj5l8jiCzJOL+A8hFO/n0tbHWt9ZZpFp9R3+c 5Y8g2AB0HKlYH9O0YH7Pa3Pl99H121vMacbW4VeCbilbT+iDrVPtteWSnN9kZFkd 6UlwGsrmlx0+sb/4MATBviv00Z2BfE3rb6YaHMryeODH+U0UMwjXqOIqzs7gqAIE Pzg+NRpdR58fwQELdz2t5ELy9Y2W90QMxqjG4SoXszUbELKShXAjAYTKLPjTVQ3C zYw8VR/ZmvEiw5ZebLfkYVKMdYrNoaNd9rkW7wWOQWUU0ERt7JFsomVODDjV4bm5 ldAlO/bKQ/J0iqdbHCGEzNF8qbG1L/Va1FpuKq6ToLhICSBZBj/x5Ns309GGHbLp Q5lLLB/+sYWg9pVYC6U= =5y1V -----END PGP SIGNATURE----- --=-=-=-- --===============0422294287== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0422294287==--