On 05/09/16 17:10, Tomi Valkeinen wrote: > 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. >
I did not really think of that. I'll make the change. > 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 = drm_atomic_helper_connector_dpms > I left that for subsequent clean up after general approach is accepted. > 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. > As modeset_nofb is the preferred call back for setting the mode, and it does not require any fb or plane, I needed something to express the mandatory framebuffer. The primatry plane just wast the first solution that came to my mind and it appeared to work. > 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 > -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: OpenPGP digital signature URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160510/0f1f2400/attachment.sig>
