[PATCH RFC 00/11] drm/tilcdc: Atomic modeset support

[Jyri Sarha]

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>