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

From: "Noralf Trønnes" <noralf@tronnes.org>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Jyri Sarha <jsarha@ti.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH RFC 00/11] drm/tilcdc: Atomic modeset support
Date: Tue, 10 May 2016 19:30:22 +0200	[thread overview]
Message-ID: <96db18a0-03f6-70d8-ec55-6a7b59d935bd@tronnes.org> (raw)
In-Reply-To: <CAKMK7uEM0pbcWGrvoqZ_wkQ_Ch1HZtO3Uznd5Ls7qQOkbV82qA@mail.gmail.com>

Den 10.05.2016 16:18, skrev Daniel Vetter:
> On Tue, May 10, 2016 at 4:04 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
>> Den 10.05.2016 11:14, skrev Jyri Sarha:
>>> On 05/10/16 09:34, Daniel Vetter wrote:
>>>>>> There's a drm_simple_display_pipe floating around which seems perfectly
>>>>>>>> suited to tilcdc. It's meant for the case where you have 1 plane, 1
>>>>>>>> crtc
>>>>>>>> and 1 encoder maybe linking to different connectors. And it takes
>>>>>>>> care of
>>>>>>>> all the small bits for you, with a grand total of 5 callbacks, all of
>>>>>>>> them
>>>>>>>> optional.
>>>>>>>>
>>>>>>>> Might indeed be useful to rebase tilcdc on top of that, should be
>>>>>>>> possible
>>>>>>>> to nuke piles of code.
>>>>>>
>>>>>> Looks interesting. Does it look like it is getting ready to be merged
>>>>>> soon?
>>>> Should be landind soon, yes. Probably not for 4.7, that's closed now, but
>>>> I can still pick it up into drm-misc right away when it's ready. Open
>>>> review comments are all just small things, you could pick the latest
>>>> version to start prototyping a conversion and there shouldn't be any
>>>> surprises when you rebase onto the merged version.
>>> Hmmm, too bad it wants to own its encoder. LCDC on Beaglebone-Black
>>> needs the componentized external tda998x driver. So at least in its
>>> current form the drm_simple_display_pipe wont work for tilcdc.
>>>
>>> It may not be too big a job to add an external encoder support, but
>>> would it complicate currently so nice and simple driver structure too
>>> much?
>>
>> How about we add an argument for encoder and if it is NULL, then the no-op
>> encoder is used:
> Hm, my idea with external transcoders was to pull them in as a
> drm_bridge. That's of course more work, and we already have a
> proliferation of different transcoder driver standards in drm
> unfortunately (there's drm_bridge, but als to drm_encoder_slave).

Does this mean that you want the drm_bridge_*() functions in
disable_outputs(), crtc_set_mode() and
drm_atomic_helper_commit_modeset_enables() to run for 
drm_simple_display_pipe?

Because "drm: Make drm_encoder_helper_funcs optional" skips those bridge
functions if encoder->helper_private is not set (I found this pattern in
mode_fixup() which skips drm_bridge_mode_fixup() on !funcs).
So if that's the case, I have to add an empty drm_encoder_helper_funcs 
to the
simple pipe.

Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel