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

[Daniel Vetter]

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.

Thanks, Daniel
> 
> Mark
> ________________________________________
> From: Daniel Vetter <daniel.vetter at ffwll.ch> on behalf of Daniel Vetter <daniel at ffwll.ch>
> Sent: Monday, May 11, 2015 5:27 PM
> To: Mark Zhang
> Cc: thierry.reding at gmail.com; linux-tegra at vger.kernel.org; dri-devel at 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 at 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