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