From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [RFCv1 10/12] drm: convert crtc to properties/state Date: Mon, 7 Oct 2013 17:19:38 +0300 Message-ID: <20131007141938.GM9395@intel.com> References: <1381020350-1125-1-git-send-email-robdclark@gmail.com> <1381020350-1125-11-git-send-email-robdclark@gmail.com> <20131007133936.GH9395@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 020EAE5FE2 for ; Mon, 7 Oct 2013 07:19:41 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Rob Clark Cc: "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org On Mon, Oct 07, 2013 at 10:03:01AM -0400, Rob Clark wrote: > On Mon, Oct 7, 2013 at 9:39 AM, Ville Syrj=E4l=E4 > 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(). > = > > 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. > = > 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=E4l=E4 > > Intel OTC -- = Ville Syrj=E4l=E4 Intel OTC