Re: [RFC PATCH 01/12] drm: Add a new mode flag: DRM_MODE_FLAG_PREFER_ONE_SHOT

From: Thierry Reding <thierry.reding@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	Mark Zhang <markz@nvidia.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [RFC PATCH 01/12] drm: Add a new mode flag: DRM_MODE_FLAG_PREFER_ONE_SHOT
Date: Tue, 12 May 2015 10:23:01 +0200	[thread overview]
Message-ID: <20150512082300.GA2386@ulmo> (raw)
In-Reply-To: <20150512063558.GP15256@phenom.ffwll.local>

[-- Attachment #1.1: Type: text/plain, Size: 3078 bytes --]

On Tue, May 12, 2015 at 08:35:58AM +0200, Daniel Vetter wrote:
> On Mon, May 11, 2015 at 04:34:57PM +0000, Mark Zhang wrote:
> > I just want to make things easier. If we adding this in panel's meta
> > data, it will be harder to make crtc gets this, since normally encoder
> > talks with panel and crtc talks with encoder. But yes, adding this in
> > panel's metadata makes more sense so if there is a better way to do
> > that, I'm happy to do the changes.
> 
> Adding something to the userspace ABI (which you've done here) because the
> kernel-internals are designed in an awkward way right now is definitely
> the wrong thing to do. With atomic you can easily add a bool
> prefer_oneshot to drm_crtc_state to encode this. But I fear that with the
> plain crtc helpers this just doesn't work properly. You could add a
> driver-private internal in drm_display_mode->private_flags, but that might
> clash with drivers existing use of this field.
> 
> In any way, this is definitely not something to add to uapi headers. Hence
> Nacked-by: me.

Are there use-cases where one-shot mode is worse than continuous mode?
I'm thinking games that run at full FPS and such. If so, exposing this
to userspace is perhaps not a bad idea, albeit not via a mode flag. If
userspace had a way to set the preference, it could do so depending on
use-case.

Thierry

> > ________________________________________
> > From: Daniel Vetter <daniel.vetter@ffwll.ch> on behalf of Daniel Vetter <daniel@ffwll.ch>
> > Sent: Monday, May 11, 2015 5:27 PM
> > To: Mark Zhang
> > Cc: thierry.reding@gmail.com; linux-tegra@vger.kernel.org; dri-devel@lists.freedesktop.org
> > Subject: Re: [RFC PATCH 01/12] drm: Add a new mode flag: DRM_MODE_FLAG_PREFER_ONE_SHOT
> > 
> > On Mon, May 11, 2015 at 09:38:20AM +0800, Mark Zhang wrote:
> > > Normally this flag is set by panel driver so that crtc can enable
> > > the "one-shot" mode(not scan frames continuously).
> > >
> > > Signed-off-by: Mark Zhang <markz@nvidia.com>
> > > ---
> > >  include/uapi/drm/drm_mode.h | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > > index dbeba949462a..5447a338e893 100644
> > > --- a/include/uapi/drm/drm_mode.h
> > > +++ b/include/uapi/drm/drm_mode.h
> > > @@ -72,6 +72,7 @@
> > >  #define  DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH      (6<<14)
> > >  #define  DRM_MODE_FLAG_3D_TOP_AND_BOTTOM     (7<<14)
> > >  #define  DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF  (8<<14)
> > > +#define DRM_MODE_FLAG_PREFER_ONE_SHOT                (1<<19)
> > 
> > tbh this doesn't sound like a mode flag, but something which should be
> > attached to the drm_panel. Especially since all the single-frame modes are
> > highly sink/link specific. Why was this added here instead of to the
> > drm_panel metadata?
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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