[RFCv1 10/12] drm: convert crtc to properties/state

[Rob Clark]

On Mon, Oct 7, 2013 at 10:19 AM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> On Mon, Oct 07, 2013 at 10:03:01AM -0400, Rob Clark wrote:
>> On Mon, Oct 7, 2013 at 9:39 AM, Ville Syrjälä
>> <ville.syrjala at linux.intel.com> wrote:
>> > On Sat, Oct 05, 2013 at 08:45:48PM -0400, Rob Clark wrote:
>> >> Break the mutable state of a crtc out into a separate structure
>> >> and use atomic properties mechanism to set crtc attributes.  This
>> >> makes it easier to have some helpers for crtc->set_property()
>> >> and for checking for invalid params.  The idea is that individual
>> >> drivers can wrap the state struct in their own struct which adds
>> >> driver specific parameters, for easy build-up of state across
>> >> multiple set_property() calls and for easy atomic commit or roll-
>> >> back.
>> >
>> > I'm not sure how we're going to handle the mismatch in the behaviour of
>> > the atomic modeset vs. the current setcrtc.
>> >
>> > The problem is that setcrtc ignore all kinds of conflicting
>> > crtc->connector assignments, and just overwrites whatever was there
>> > with the latest configuration. For the atomic case we want to return an
>> > error if there's a conflict.
>>
>> Hmm, well currently we preserve the setcrtc behavior because it ends
>> up going through crtc helpers (or whatever the driver uses).  So
>> should be fine for setcrtc, but probably not what we want for atomic
>> ioctl.
>>
>> I suppose we could solve some of this via internal flags, ie
>> .atomic_begin(dev, LEGACY_SETCRTC_CHECK_MODE)
>>
>> it is a bit ugly, but it keeps the ugly in core and drivers don't have
>> to care as much about it (which is my main concern)
>
> Well, it could be an entirely separate .legacy_crap() hook or something
> that happens just before .check().

yeah, that could work too.. I'll think about it a bit and see what I
can come up with

>>
>> > And another thing is the DPMS handling. The
>> > current API forces DPMS on when you do a modeset, but for the atomic
>> > case I want to keep things nice and clean and avoid doing such silly
>> > things.
>>
>> I guess the easy thing is to set DPMS property in setcrtc too ;-)
>
> That's what we do, but I don't want it for atomic.

yeah, I need think about how that could work if we are still using
.set_config() from the commit, but I guess I should be able to sort
out something..

BR,
-R

>>
>> BR,
>> -R
>>
>> > So I don't think we can simply convert the current modeset codepaths to
>> > call into the atomic code. We basically need another version of the
>> > check function, or another step that happens before .check only in the
>> > setcrtc case which eliminates the conflicts in a way that matches the
>> > current setcrtc behaviour.
>> >
>> > --
>> > Ville Syrjälä
>> > Intel OTC
>
> --
> Ville Syrjälä
> Intel OTC