From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH 2/3] drm/omap: fix enabling/disabling of video pipeline Date: Thu, 3 Apr 2014 18:37:59 +0300 Message-ID: <533D8057.6020405@ti.com> References: <1396532753-26108-1-git-send-email-tomi.valkeinen@ti.com> <1396532753-26108-2-git-send-email-tomi.valkeinen@ti.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0605494062==" Return-path: Received: from comal.ext.ti.com (comal.ext.ti.com [198.47.26.152]) by gabe.freedesktop.org (Postfix) with ESMTP id 21EB96E026 for ; Thu, 3 Apr 2014 08:38:04 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Clark Cc: "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org --===============0605494062== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="vqXeofDstgqLrKOaI5LSq4WRLEUlCwuR3" --vqXeofDstgqLrKOaI5LSq4WRLEUlCwuR3 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 03/04/14 17:58, Rob Clark wrote: > On Thu, Apr 3, 2014 at 9:45 AM, Tomi Valkeinen = wrote: >> At the moment the omap_crtc_pre_apply() handles the enabling, disablin= g >> and configuring of encoders and panels separately from the CRTC (i.e. >> the overlay manager). >> >> However, this doesn't work correctly. The encoder driver has to be in >> control of its video input (i.e. the crtc) for correct operation. >> >> This problem causes bugs with (at least) HDMI: the HDMI encoder suppli= es >> pixel clock for DISPC, and DISPC supplies video stream for HDMI. The >> current code first enables the HDMI encoder, and CRTC after that. >> However, the encoder expects the video stream to start during the >> encoder's enable, and if it doesn't, there will be sync lost errors. >> >> The encoder enables its video source by calling src->enable(), and thi= s >> call goes to omapdrm (omap_crtc_enable), but omapdrm doesn't do anythi= ng >> in that function. Similarly for disable, which goes to >> omap_crtc_disable(). >> >> This patch moves the code to setup and enable/disable the crtc to >> omap_crtc_enable. and omap_crtc_disable(). >> >> Signed-off-by: Tomi Valkeinen >=20 > I had to go back to look at the code to realize the extra > 'omap_crtc->full_update =3D false' removal didn't actually matter (it > was redundant). So The final 'omap_crtc->full_update =3D false;' is visible in the patch below also, so you didn't need to go look at the code ;) Looking at it, that removal is actually extra, not exactly part of this f= ix. > Reviewed-by: Rob Clark Thanks! Tom >=20 >> --- >> drivers/gpu/drm/omapdrm/omap_crtc.c | 19 ++++++++++++------- >> 1 file changed, 12 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/oma= pdrm/omap_crtc.c >> index beccff2ccf84..90916abb66ef 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c >> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c >> @@ -121,13 +121,25 @@ static void omap_crtc_start_update(struct omap_o= verlay_manager *mgr) >> { >> } >> >> +static void set_enabled(struct drm_crtc *crtc, bool enable); >> + >> static int omap_crtc_enable(struct omap_overlay_manager *mgr) >> { >> + struct omap_crtc *omap_crtc =3D omap_crtcs[mgr->id]; >> + >> + dispc_mgr_setup(omap_crtc->channel, &omap_crtc->info); >> + dispc_mgr_set_timings(omap_crtc->channel, >> + &omap_crtc->timings); >> + set_enabled(&omap_crtc->base, true); >> + >> return 0; >> } >> >> static void omap_crtc_disable(struct omap_overlay_manager *mgr) >> { >> + struct omap_crtc *omap_crtc =3D omap_crtcs[mgr->id]; >> + >> + set_enabled(&omap_crtc->base, false); >> } >> >> static void omap_crtc_set_timings(struct omap_overlay_manager *mgr, >> @@ -600,7 +612,6 @@ static void omap_crtc_pre_apply(struct omap_drm_ap= ply *apply) >> omap_crtc->current_encoder =3D encoder; >> >> if (!omap_crtc->enabled) { >> - set_enabled(&omap_crtc->base, false); >> if (encoder) >> omap_encoder_set_enabled(encoder, false); >> } else { >> @@ -609,13 +620,7 @@ static void omap_crtc_pre_apply(struct omap_drm_a= pply *apply) >> omap_encoder_update(encoder, omap_crtc->mgr, >> &omap_crtc->timings); >> omap_encoder_set_enabled(encoder, true); >> - omap_crtc->full_update =3D false; >> } >> - >> - dispc_mgr_setup(omap_crtc->channel, &omap_crtc->info);= >> - dispc_mgr_set_timings(omap_crtc->channel, >> - &omap_crtc->timings); >> - set_enabled(&omap_crtc->base, true); >> } >> >> omap_crtc->full_update =3D false; >> -- >> 1.8.3.2 >> --vqXeofDstgqLrKOaI5LSq4WRLEUlCwuR3 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQIcBAEBAgAGBQJTPYBXAAoJEPo9qoy8lh71PMYP/2eJaL0n/h6sWe8PECP6/hJa h2SgsVqyZ148/23VAwhghTksqtT/khFRXwOZsETZ0hhdZSTba8Uy9FsIHk0vhCtQ hqt5uMYnqi3ryzTUoVsbuP4/6XJbvjLGsC5MnlD4AI7RewHj1dfbtLPdU4L+8rkl IsjLJTq8wL+/Z+FNZp8AFtdsCZ5dVB+ucLEE9hc/vii85UKrECPrQTCMqyzCQy2I bHO1xzCZLYMuTkl6cc2Rm/oduX1cI/hr77d3MqCd7OxLwnE+hMhxrm+EAvPEfwDD 6Ekg17Xo3OGSD4zl3xupO5FypWpfOqCBruB4fHtMB866TNy7IWBYctgi4lu8ydJz Knvck9f1QsGHS6Fx2mlvAgx4Qm08Ms4iNmdjkJTjBiFmcg9VdxmEMMAAdE/S3e6/ pq3xsWsxDjozHV+9jMIKHH2TK5GnfB0Z0nN/MhCJRFFok0SX3TVCBTqCtp9RtDJo NmeWNsWs2m4AbYJO6IE1WS1UcGQ5psf+PcW3VKzCsqnoHygewIpFCVajsNtxWWRw S3H3yLOlVnn2VOX+cSQ9EGjzW9g6CCbXTEf+1DBSjbCG3GAyTJK+nrQN8bp9+odD CQL4lt9bNRNy3GuqTllNAp4rPQuDfVbeoL5qQAzNR1oo38i8vyzR+kAc6DWHAn0Y PCzs1rcDwWRa4n1mPC+0 =3uXO -----END PGP SIGNATURE----- --vqXeofDstgqLrKOaI5LSq4WRLEUlCwuR3-- --===============0605494062== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============0605494062==--