From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH RFC 00/11] drm/tilcdc: Atomic modeset support Date: Mon, 9 May 2016 17:10:00 +0300 Message-ID: <57309A38.6000702@ti.com> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1768877131==" Return-path: Received: from bear.ext.ti.com (bear.ext.ti.com [192.94.94.41]) by gabe.freedesktop.org (Postfix) with ESMTPS id E1AD96E2D6 for ; Mon, 9 May 2016 14:10:08 +0000 (UTC) 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: Jyri Sarha , dri-devel@lists.freedesktop.org Cc: laurent.pinchart@ideasonboard.com List-Id: dri-devel@lists.freedesktop.org --===============1768877131== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hxVeXe0I0ld7JnkgNu6Ia8CFNfpGuWj9v" --hxVeXe0I0ld7JnkgNu6Ia8CFNfpGuWj9v Content-Type: multipart/mixed; boundary="FxmfkdOi79gGDWoSFRknJKtUxhGmn7hWr" From: Tomi Valkeinen To: Jyri Sarha , dri-devel@lists.freedesktop.org Cc: airlied@linux.ie, daniel@ffwll.ch, laurent.pinchart@ideasonboard.com, robdclark@gmail.com Message-ID: <57309A38.6000702@ti.com> Subject: Re: [PATCH RFC 00/11] drm/tilcdc: Atomic modeset support References: In-Reply-To: --FxmfkdOi79gGDWoSFRknJKtUxhGmn7hWr Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Jyri, On 11/04/16 19:46, Jyri Sarha wrote: > The LCDC in its simplicity does not fit too well into DRM atomic > modeset abstractions. I wonder if I am doing the right thing in > implementing the dummy primary plane and in implementing > mode_set_nofb() crtc helper when the crtc actually needs the > framebuffer to be there when configuring it. See individual patch > descriptions for details. There is still lot of room for cleaning up > but I would first like to know if I am moving at all to the right > direction. I'm no expert on atomic modesetting, but here are my comments/questions: You add drm_crtc_vblank_off() to crtc_destroy() callback. Why is that? I think you should call drm_crtc_vblank_on() in crtc_enable(), and vblank_off in disable. You remove the setting of tilcdc_crtc_helper_funcs.dpms, but leave the tilcdc_crtc_dpms, which you use elsewhere. I think all dpms stuff should be removed from crtc, as it's all either "enable" or "disable". Git grep shows that in omapdrm, there's just one reference to dpms, in omap_connector.c: .dpms =3D drm_atomic_helper_connector_dpms It's not clear to me if a (primary) plane is required with atomic universal planes and modesetting or not... I guess it is, when thinking of how e.g. a framebuffer is configured to be shown on a screen when using the atomic modesetting uapi. But if it is required, it makes me wonder, are there other HWs out there without any planes? The dummy plane implementation you added is not complex, but is it something that should be implemented with DRM helper funcs? Tomi --FxmfkdOi79gGDWoSFRknJKtUxhGmn7hWr-- --hxVeXe0I0ld7JnkgNu6Ia8CFNfpGuWj9v Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIbBAEBCAAGBQJXMJo4AAoJEPo9qoy8lh71EH4P+Ld37nT84Bfkg7frmkgH7oyo 1LTEr0/k8XO5hOp7mdfAoV41+/7elX/83oQQOp8BS+9/SHlVTljDQ9R3oGQrMX+j eo2wzpPWBVqAFKipmf5dDD/zYgGAg7zue86bIPeFmukGKjy5u52uiVpd6R2QbjOJ Ze5QS7Q3pe9qC9D7aT9AtDNdCQdifu+egQi8uXirEuBNu0L5naM+wBf9XnvvBQwY STEgcEXu8K2V3avBfNbiYVyeReAJbusEj0eAdhbLoRzA3a9HG+ro5dUYLRJUYo2y 5OeQZrKBYDGxItWoFyerNQSwRRP0bwMYYvoUi0ZTWTaYUFtEyPl+yQ52TFTnkusZ sE6QfKbjV+wTFGfVey1SdvHWKvfCYzuyQO7m9cSBYxOL864wm9v3BEfLslWbURni PQkig3g6ODf+AWbZKL3vilqP1UB3m1M4BIGF+2eFiW9wdeR8rYuaOk2utG8jOxHo QCQv7hxvn/l5xTlMbU+AbCukkMazJCg9cJBHe4alTaCjsxG9j8op5QH7WgOrYdTy 0lUEHBG+ENypSmZVE2NS0a4sKOju+YrVVqiHAc+GOZKsJmBBLVxM1IYHUxt2qzlq q+D6NXZYgf4mHFDGq+4Wce1l71FrNylGG9DuqyLL+XlQTrZ1oAk4pw9JWsHDAe14 4Z6ncMbpPQsBJFq1haE= =QtCh -----END PGP SIGNATURE----- --hxVeXe0I0ld7JnkgNu6Ia8CFNfpGuWj9v-- --===============1768877131== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1768877131==--