Re: [RFC 1/4] drm: Add writeback connector type

[Sean Paul]

On Fri, Feb 23, 2018 at 04:48:58PM +, Liviu Dudau wrote:
> On Fri, Feb 23, 2018 at 11:43:29AM -0500, Sean Paul wrote:
> > On Fri, Feb 23, 2018 at 11:25:11AM -0500, Rob Clark wrote:
> > > On Fri, Feb 23, 2018 at 10:59 AM, Sean Paul  wrote:
> > > >
> > > > Have we considered hiding writeback behind a client cap instead?
> > > 
> > > It is kinda *almost* unneeded, since the connector reports itself as
> > > disconnected.
> > > 
> > > I'm not sure what the reason was to drop the cap, but I think it would
> > > be better to have a cap so WB connectors don't show up in, for ex,
> > > xrandr
> > 
> > Yeah, the disconnected hack is kind of gross, IMO. I hate to introduce 
> > churn in
> > the patch series given that it was initially introduced with the client cap.
> 
> Haha, that's the reverse of Daniel's position:
> 
> https://lists.freedesktop.org/archives/dri-devel/2016-October/120519.html

Yeah, it happens :(. I don't think it's a dealbreaker either way, it just seems
awkward to expose a connector which is "disconnected", but available for use. I
don't think we have any other connectors which are supposed to be used in this
state.

> 
> > 
> > There are also cases where we might want to make writeback unavailable, 
> > such as
> > when content protection is enabled. In those cases, it's conceivable that we
> > might want to use disconnected as a signal to u/s. I suppose we could also 
> > just
> > fail the check, so most of this is just academic.
> 
> Not sure what other hardware out there does, but on Mali DP's case you
> would be outputing the protected content by putting the display
> processor in secure mode, which automatically disables writeback for us.
> Or to put in another way, you don't need a writeback framebuffer if you
> are in non-secure mode as you can get access to the framebuffer used for
> the plane anyway.

Yeah, I was mostly thinking about the case where you might have HDCP enabled on
the HDMI connector and be able to slurp up the content via a writeback. However
if the buffer is not secure in the first place, then it's already accessible in
userspace so I don't think that writeback presents a new security hole.

/me needs to get HDCP out of his head.

Sean


> 
> Best regards,
> Liviu
> 
> > 
> > Sean
> > 
> > 
> > > 
> > > BR,
> > > -R
> > 
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> 
> -- 
> 
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---
> ¯\_(ツ)_/¯

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/4] drm: Add writeback connector type

[Liviu Dudau]

On Fri, Feb 23, 2018 at 11:39:06AM -0500, Sean Paul wrote:
> On Fri, Feb 23, 2018 at 04:21:05PM +, Liviu Dudau wrote:
> > On Fri, Feb 23, 2018 at 10:59:35AM -0500, Sean Paul wrote:
> > > On Fri, Feb 23, 2018 at 08:17:51AM -0500, Rob Clark wrote:
> > > > From: Brian Starkey 
> > > > 
> > > > Writeback connectors represent writeback engines which can write the
> > > > CRTC output to a memory framebuffer. Add a writeback connector type and
> > > > related support functions.
> > > > 
> > > > Drivers should initialize a writeback connector with
> > > > drm_writeback_connector_init() which takes care of setting up all the
> > > > writeback-specific details on top of the normal functionality of
> > > > drm_connector_init().
> > > > 
> > > > Writeback connectors have a WRITEBACK_FB_ID property, used to set the
> > > > output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose 
> > > > the
> > > > supported writeback formats to userspace.
> > > > 
> > > > When a framebuffer is attached to a writeback connector with the
> > > > WRITEBACK_FB_ID property, it is used only once (for the commit in which
> > > > it was included), and userspace can never read back the value of
> > > > WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
> > > > attached to a CRTC.
> > > > 
> > > > Changes since v1:
> > > >  - Added drm_writeback.c + documentation
> > > >  - Added helper to initialize writeback connector in one go
> > > >  - Added core checks
> > > >  - Squashed into a single commit
> > > >  - Dropped the client cap
> > > >  - Writeback framebuffers are no longer persistent
> > > > 
> > > > Changes since v2:
> > > >  Daniel Vetter:
> > > >  - Subclass drm_connector to drm_writeback_connector
> > > >  - Relax check to allow CRTC to be set without an FB
> > > >  - Add some writeback_ prefixes
> > > >  - Drop PIXEL_FORMATS_SIZE property, as it was unnecessary
> > > >  Gustavo Padovan:
> > > >  - Add drm_writeback_job to handle writeback signalling centrally
> > > > 
> > > > Changes since v3:
> > > >  - Rebased
> > > >  - Rename PIXEL_FORMATS -> WRITEBACK_PIXEL_FORMATS
> > > > 
> > > > Changes since v4:
> > > >  - Added atomic_commit() vfunc to connector helper funcs, so that
> > > >writeback jobs are committed from atomic helpers
> > > > 
> > > > Signed-off-by: Brian Starkey 
> > > > [rebased and fixed conflicts]
> > > > Signed-off-by: Mihail Atanassov 
> > > > Signed-off-by: Liviu Dudau 
> > > > [rebased and added atomic_commit() vfunc for writeback jobs]
> > > > Signed-off-by: Rob Clark 
> > > > ---
> > > >  Documentation/gpu/drm-kms.rst|   9 ++
> > > >  drivers/gpu/drm/Makefile |   2 +-
> > > >  drivers/gpu/drm/drm_atomic.c | 130 
> > > >  drivers/gpu/drm/drm_atomic_helper.c  |  30 
> > > >  drivers/gpu/drm/drm_connector.c  |   4 +-
> > > >  drivers/gpu/drm/drm_writeback.c  | 257 
> > > > +++
> > > >  include/drm/drm_atomic.h |   3 +
> > > >  include/drm/drm_connector.h  |  13 ++
> > > >  include/drm/drm_mode_config.h|  14 ++
> > > >  include/drm/drm_modeset_helper_vtables.h |  11 ++
> > > >  include/drm/drm_writeback.h  |  89 +++
> > > >  include/uapi/drm/drm_mode.h  |   1 +
> > > >  12 files changed, 561 insertions(+), 2 deletions(-)
> > > >  create mode 100644 drivers/gpu/drm/drm_writeback.c
> > > >  create mode 100644 include/drm/drm_writeback.h
> > > > 
> > > > diff --git a/Documentation/gpu/drm-kms.rst 
> > > > b/Documentation/gpu/drm-kms.rst
> > > > index 2dcf5b42015d..e7590dd2f71e 100644
> > > > --- a/Documentation/gpu/drm-kms.rst
> > > > +++ b/Documentation/gpu/drm-kms.rst
> > > > @@ -370,6 +370,15 @@ Connector Functions Reference
> > > >  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > > > :export:
> > > >  
> > > > +Writeback Connectors
> > > > +
> > > > +
> > > > +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> > > > +  :doc: overview
> > > > +
> > > > +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> > > > +  :export:
> > > > +
> > > >  Encoder Abstraction
> > > >  ===
> > > >  
> > > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > > index 50093ff4479b..3d708959b224 100644
> > > > --- a/drivers/gpu/drm/Makefile
> > > > +++ b/drivers/gpu/drm/Makefile
> > > > @@ -18,7 +18,7 @@ drm-y   :=drm_auth.o drm_bufs.o 
> > > > drm_cache.o \
> > > > drm_encoder.o drm_mode_object.o drm_property.o \
> > > > drm_plane.o drm_color_mgmt.o drm_print.o \
> > > > drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
> > > > -   drm_syncobj.o drm_lease.o
> > > > +   drm_syncobj.o drm_lease.o drm_writeback.o
> > > >  
> > > >  drm-$(CONFIG_DRM_LIB_RANDOM) += 

Re: [RFC 1/4] drm: Add writeback connector type

[Liviu Dudau]

On Fri, Feb 23, 2018 at 11:43:29AM -0500, Sean Paul wrote:
> On Fri, Feb 23, 2018 at 11:25:11AM -0500, Rob Clark wrote:
> > On Fri, Feb 23, 2018 at 10:59 AM, Sean Paul  wrote:
> > >
> > > Have we considered hiding writeback behind a client cap instead?
> > 
> > It is kinda *almost* unneeded, since the connector reports itself as
> > disconnected.
> > 
> > I'm not sure what the reason was to drop the cap, but I think it would
> > be better to have a cap so WB connectors don't show up in, for ex,
> > xrandr
> 
> Yeah, the disconnected hack is kind of gross, IMO. I hate to introduce churn 
> in
> the patch series given that it was initially introduced with the client cap.

Haha, that's the reverse of Daniel's position:

https://lists.freedesktop.org/archives/dri-devel/2016-October/120519.html

> 
> There are also cases where we might want to make writeback unavailable, such 
> as
> when content protection is enabled. In those cases, it's conceivable that we
> might want to use disconnected as a signal to u/s. I suppose we could also 
> just
> fail the check, so most of this is just academic.

Not sure what other hardware out there does, but on Mali DP's case you
would be outputing the protected content by putting the display
processor in secure mode, which automatically disables writeback for us.
Or to put in another way, you don't need a writeback framebuffer if you
are in non-secure mode as you can get access to the framebuffer used for
the plane anyway.

Best regards,
Liviu

> 
> Sean
> 
> 
> > 
> > BR,
> > -R
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/4] drm: Add writeback connector type

[Sean Paul]

On Fri, Feb 23, 2018 at 11:25:11AM -0500, Rob Clark wrote:
> On Fri, Feb 23, 2018 at 10:59 AM, Sean Paul  wrote:
> >
> > Have we considered hiding writeback behind a client cap instead?
> 
> It is kinda *almost* unneeded, since the connector reports itself as
> disconnected.
> 
> I'm not sure what the reason was to drop the cap, but I think it would
> be better to have a cap so WB connectors don't show up in, for ex,
> xrandr

Yeah, the disconnected hack is kind of gross, IMO. I hate to introduce churn in
the patch series given that it was initially introduced with the client cap.

There are also cases where we might want to make writeback unavailable, such as
when content protection is enabled. In those cases, it's conceivable that we
might want to use disconnected as a signal to u/s. I suppose we could also just
fail the check, so most of this is just academic.

Sean


> 
> BR,
> -R

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/4] drm: Add writeback connector type

[Sean Paul]

On Fri, Feb 23, 2018 at 04:21:05PM +, Liviu Dudau wrote:
> On Fri, Feb 23, 2018 at 10:59:35AM -0500, Sean Paul wrote:
> > On Fri, Feb 23, 2018 at 08:17:51AM -0500, Rob Clark wrote:
> > > From: Brian Starkey 
> > > 
> > > Writeback connectors represent writeback engines which can write the
> > > CRTC output to a memory framebuffer. Add a writeback connector type and
> > > related support functions.
> > > 
> > > Drivers should initialize a writeback connector with
> > > drm_writeback_connector_init() which takes care of setting up all the
> > > writeback-specific details on top of the normal functionality of
> > > drm_connector_init().
> > > 
> > > Writeback connectors have a WRITEBACK_FB_ID property, used to set the
> > > output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose the
> > > supported writeback formats to userspace.
> > > 
> > > When a framebuffer is attached to a writeback connector with the
> > > WRITEBACK_FB_ID property, it is used only once (for the commit in which
> > > it was included), and userspace can never read back the value of
> > > WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
> > > attached to a CRTC.
> > > 
> > > Changes since v1:
> > >  - Added drm_writeback.c + documentation
> > >  - Added helper to initialize writeback connector in one go
> > >  - Added core checks
> > >  - Squashed into a single commit
> > >  - Dropped the client cap
> > >  - Writeback framebuffers are no longer persistent
> > > 
> > > Changes since v2:
> > >  Daniel Vetter:
> > >  - Subclass drm_connector to drm_writeback_connector
> > >  - Relax check to allow CRTC to be set without an FB
> > >  - Add some writeback_ prefixes
> > >  - Drop PIXEL_FORMATS_SIZE property, as it was unnecessary
> > >  Gustavo Padovan:
> > >  - Add drm_writeback_job to handle writeback signalling centrally
> > > 
> > > Changes since v3:
> > >  - Rebased
> > >  - Rename PIXEL_FORMATS -> WRITEBACK_PIXEL_FORMATS
> > > 
> > > Changes since v4:
> > >  - Added atomic_commit() vfunc to connector helper funcs, so that
> > >writeback jobs are committed from atomic helpers
> > > 
> > > Signed-off-by: Brian Starkey 
> > > [rebased and fixed conflicts]
> > > Signed-off-by: Mihail Atanassov 
> > > Signed-off-by: Liviu Dudau 
> > > [rebased and added atomic_commit() vfunc for writeback jobs]
> > > Signed-off-by: Rob Clark 
> > > ---
> > >  Documentation/gpu/drm-kms.rst|   9 ++
> > >  drivers/gpu/drm/Makefile |   2 +-
> > >  drivers/gpu/drm/drm_atomic.c | 130 
> > >  drivers/gpu/drm/drm_atomic_helper.c  |  30 
> > >  drivers/gpu/drm/drm_connector.c  |   4 +-
> > >  drivers/gpu/drm/drm_writeback.c  | 257 
> > > +++
> > >  include/drm/drm_atomic.h |   3 +
> > >  include/drm/drm_connector.h  |  13 ++
> > >  include/drm/drm_mode_config.h|  14 ++
> > >  include/drm/drm_modeset_helper_vtables.h |  11 ++
> > >  include/drm/drm_writeback.h  |  89 +++
> > >  include/uapi/drm/drm_mode.h  |   1 +
> > >  12 files changed, 561 insertions(+), 2 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/drm_writeback.c
> > >  create mode 100644 include/drm/drm_writeback.h
> > > 
> > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > > index 2dcf5b42015d..e7590dd2f71e 100644
> > > --- a/Documentation/gpu/drm-kms.rst
> > > +++ b/Documentation/gpu/drm-kms.rst
> > > @@ -370,6 +370,15 @@ Connector Functions Reference
> > >  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > > :export:
> > >  
> > > +Writeback Connectors
> > > +
> > > +
> > > +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> > > +  :doc: overview
> > > +
> > > +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> > > +  :export:
> > > +
> > >  Encoder Abstraction
> > >  ===
> > >  
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > index 50093ff4479b..3d708959b224 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -18,7 +18,7 @@ drm-y   :=  drm_auth.o drm_bufs.o drm_cache.o \
> > >   drm_encoder.o drm_mode_object.o drm_property.o \
> > >   drm_plane.o drm_color_mgmt.o drm_print.o \
> > >   drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
> > > - drm_syncobj.o drm_lease.o
> > > + drm_syncobj.o drm_lease.o drm_writeback.o
> > >  
> > >  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> > >  drm-$(CONFIG_DRM_VM) += drm_vm.o
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 46733d534587..019f131fe8be 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -30,6 +30,7 @@
> > >  #include 
> > >  #include 
> > >  

Re: [RFC 1/4] drm: Add writeback connector type

[Rob Clark]

On Fri, Feb 23, 2018 at 10:59 AM, Sean Paul  wrote:
>
> Have we considered hiding writeback behind a client cap instead?

It is kinda *almost* unneeded, since the connector reports itself as
disconnected.

I'm not sure what the reason was to drop the cap, but I think it would
be better to have a cap so WB connectors don't show up in, for ex,
xrandr

BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/4] drm: Add writeback connector type

[Liviu Dudau]

On Fri, Feb 23, 2018 at 10:59:35AM -0500, Sean Paul wrote:
> On Fri, Feb 23, 2018 at 08:17:51AM -0500, Rob Clark wrote:
> > From: Brian Starkey 
> > 
> > Writeback connectors represent writeback engines which can write the
> > CRTC output to a memory framebuffer. Add a writeback connector type and
> > related support functions.
> > 
> > Drivers should initialize a writeback connector with
> > drm_writeback_connector_init() which takes care of setting up all the
> > writeback-specific details on top of the normal functionality of
> > drm_connector_init().
> > 
> > Writeback connectors have a WRITEBACK_FB_ID property, used to set the
> > output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose the
> > supported writeback formats to userspace.
> > 
> > When a framebuffer is attached to a writeback connector with the
> > WRITEBACK_FB_ID property, it is used only once (for the commit in which
> > it was included), and userspace can never read back the value of
> > WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
> > attached to a CRTC.
> > 
> > Changes since v1:
> >  - Added drm_writeback.c + documentation
> >  - Added helper to initialize writeback connector in one go
> >  - Added core checks
> >  - Squashed into a single commit
> >  - Dropped the client cap
> >  - Writeback framebuffers are no longer persistent
> > 
> > Changes since v2:
> >  Daniel Vetter:
> >  - Subclass drm_connector to drm_writeback_connector
> >  - Relax check to allow CRTC to be set without an FB
> >  - Add some writeback_ prefixes
> >  - Drop PIXEL_FORMATS_SIZE property, as it was unnecessary
> >  Gustavo Padovan:
> >  - Add drm_writeback_job to handle writeback signalling centrally
> > 
> > Changes since v3:
> >  - Rebased
> >  - Rename PIXEL_FORMATS -> WRITEBACK_PIXEL_FORMATS
> > 
> > Changes since v4:
> >  - Added atomic_commit() vfunc to connector helper funcs, so that
> >writeback jobs are committed from atomic helpers
> > 
> > Signed-off-by: Brian Starkey 
> > [rebased and fixed conflicts]
> > Signed-off-by: Mihail Atanassov 
> > Signed-off-by: Liviu Dudau 
> > [rebased and added atomic_commit() vfunc for writeback jobs]
> > Signed-off-by: Rob Clark 
> > ---
> >  Documentation/gpu/drm-kms.rst|   9 ++
> >  drivers/gpu/drm/Makefile |   2 +-
> >  drivers/gpu/drm/drm_atomic.c | 130 
> >  drivers/gpu/drm/drm_atomic_helper.c  |  30 
> >  drivers/gpu/drm/drm_connector.c  |   4 +-
> >  drivers/gpu/drm/drm_writeback.c  | 257 
> > +++
> >  include/drm/drm_atomic.h |   3 +
> >  include/drm/drm_connector.h  |  13 ++
> >  include/drm/drm_mode_config.h|  14 ++
> >  include/drm/drm_modeset_helper_vtables.h |  11 ++
> >  include/drm/drm_writeback.h  |  89 +++
> >  include/uapi/drm/drm_mode.h  |   1 +
> >  12 files changed, 561 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/gpu/drm/drm_writeback.c
> >  create mode 100644 include/drm/drm_writeback.h
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 2dcf5b42015d..e7590dd2f71e 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -370,6 +370,15 @@ Connector Functions Reference
> >  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > :export:
> >  
> > +Writeback Connectors
> > +
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> > +  :doc: overview
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> > +  :export:
> > +
> >  Encoder Abstraction
> >  ===
> >  
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 50093ff4479b..3d708959b224 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -18,7 +18,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
> > drm_encoder.o drm_mode_object.o drm_property.o \
> > drm_plane.o drm_color_mgmt.o drm_print.o \
> > drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
> > -   drm_syncobj.o drm_lease.o
> > +   drm_syncobj.o drm_lease.o drm_writeback.o
> >  
> >  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
> >  drm-$(CONFIG_DRM_VM) += drm_vm.o
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 46733d534587..019f131fe8be 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -30,6 +30,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  
> >  #include "drm_crtc_internal.h"
> > @@ -638,6 +639,46 @@ static void drm_atomic_crtc_print_state(struct 
> > drm_printer *p,
> > crtc->funcs->atomic_print_state(p, state);
> >  }
> >  
> > +/**
> > + * 

Re: [RFC 1/4] drm: Add writeback connector type

[Sean Paul]

On Fri, Feb 23, 2018 at 08:17:51AM -0500, Rob Clark wrote:
> From: Brian Starkey 
> 
> Writeback connectors represent writeback engines which can write the
> CRTC output to a memory framebuffer. Add a writeback connector type and
> related support functions.
> 
> Drivers should initialize a writeback connector with
> drm_writeback_connector_init() which takes care of setting up all the
> writeback-specific details on top of the normal functionality of
> drm_connector_init().
> 
> Writeback connectors have a WRITEBACK_FB_ID property, used to set the
> output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose the
> supported writeback formats to userspace.
> 
> When a framebuffer is attached to a writeback connector with the
> WRITEBACK_FB_ID property, it is used only once (for the commit in which
> it was included), and userspace can never read back the value of
> WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
> attached to a CRTC.
> 
> Changes since v1:
>  - Added drm_writeback.c + documentation
>  - Added helper to initialize writeback connector in one go
>  - Added core checks
>  - Squashed into a single commit
>  - Dropped the client cap
>  - Writeback framebuffers are no longer persistent
> 
> Changes since v2:
>  Daniel Vetter:
>  - Subclass drm_connector to drm_writeback_connector
>  - Relax check to allow CRTC to be set without an FB
>  - Add some writeback_ prefixes
>  - Drop PIXEL_FORMATS_SIZE property, as it was unnecessary
>  Gustavo Padovan:
>  - Add drm_writeback_job to handle writeback signalling centrally
> 
> Changes since v3:
>  - Rebased
>  - Rename PIXEL_FORMATS -> WRITEBACK_PIXEL_FORMATS
> 
> Changes since v4:
>  - Added atomic_commit() vfunc to connector helper funcs, so that
>writeback jobs are committed from atomic helpers
> 
> Signed-off-by: Brian Starkey 
> [rebased and fixed conflicts]
> Signed-off-by: Mihail Atanassov 
> Signed-off-by: Liviu Dudau 
> [rebased and added atomic_commit() vfunc for writeback jobs]
> Signed-off-by: Rob Clark 
> ---
>  Documentation/gpu/drm-kms.rst|   9 ++
>  drivers/gpu/drm/Makefile |   2 +-
>  drivers/gpu/drm/drm_atomic.c | 130 
>  drivers/gpu/drm/drm_atomic_helper.c  |  30 
>  drivers/gpu/drm/drm_connector.c  |   4 +-
>  drivers/gpu/drm/drm_writeback.c  | 257 
> +++
>  include/drm/drm_atomic.h |   3 +
>  include/drm/drm_connector.h  |  13 ++
>  include/drm/drm_mode_config.h|  14 ++
>  include/drm/drm_modeset_helper_vtables.h |  11 ++
>  include/drm/drm_writeback.h  |  89 +++
>  include/uapi/drm/drm_mode.h  |   1 +
>  12 files changed, 561 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_writeback.c
>  create mode 100644 include/drm/drm_writeback.h
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 2dcf5b42015d..e7590dd2f71e 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -370,6 +370,15 @@ Connector Functions Reference
>  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> :export:
>  
> +Writeback Connectors
> +
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> +  :doc: overview
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
> +  :export:
> +
>  Encoder Abstraction
>  ===
>  
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 50093ff4479b..3d708959b224 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -18,7 +18,7 @@ drm-y   :=  drm_auth.o drm_bufs.o drm_cache.o \
>   drm_encoder.o drm_mode_object.o drm_property.o \
>   drm_plane.o drm_color_mgmt.o drm_print.o \
>   drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
> - drm_syncobj.o drm_lease.o
> + drm_syncobj.o drm_lease.o drm_writeback.o
>  
>  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>  drm-$(CONFIG_DRM_VM) += drm_vm.o
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 46733d534587..019f131fe8be 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -30,6 +30,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "drm_crtc_internal.h"
> @@ -638,6 +639,46 @@ static void drm_atomic_crtc_print_state(struct 
> drm_printer *p,
>   crtc->funcs->atomic_print_state(p, state);
>  }
>  
> +/**
> + * drm_atomic_connector_check - check connector state
> + * @connector: connector to check
> + * @state: connector state to check
> + *
> + * Provides core sanity checks for connector state.
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +static int 

Re: [RFC 1/4] drm: Add writeback connector type

[Liviu Dudau]

On Fri, Feb 23, 2018 at 09:24:10AM -0500, Rob Clark wrote:
> On Fri, Feb 23, 2018 at 9:00 AM, Liviu Dudau  wrote:
> > Hi Rob,
> >
> > On Fri, Feb 23, 2018 at 08:17:51AM -0500, Rob Clark wrote:
> >> From: Brian Starkey 
> >>
> >> Writeback connectors represent writeback engines which can write the
> >> CRTC output to a memory framebuffer. Add a writeback connector type and
> >> related support functions.
> >>
> >> Drivers should initialize a writeback connector with
> >> drm_writeback_connector_init() which takes care of setting up all the
> >> writeback-specific details on top of the normal functionality of
> >> drm_connector_init().
> >>
> >> Writeback connectors have a WRITEBACK_FB_ID property, used to set the
> >> output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose the
> >> supported writeback formats to userspace.
> >>
> >> When a framebuffer is attached to a writeback connector with the
> >> WRITEBACK_FB_ID property, it is used only once (for the commit in which
> >> it was included), and userspace can never read back the value of
> >> WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
> >> attached to a CRTC.
> >
> > [snip]
> >
> >> +static bool create_writeback_properties(struct drm_device *dev)
> >> +{
> >> + struct drm_property *prop;
> >> +
> >> + if (!dev->mode_config.writeback_fb_id_property) {
> >> + prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
> >> +   "WRITEBACK_FB_ID",
> >> +   DRM_MODE_OBJECT_FB);
> >> + if (!prop)
> >> + return false;
> >> + dev->mode_config.writeback_fb_id_property = prop;
> >> + }
> >> +
> >> + if (!dev->mode_config.writeback_pixel_formats_property) {
> >> + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB | 
> >> DRM_MODE_PROP_IMMUTABLE,
> >> +"WRITEBACK_PIXEL_FORMATS", 0);
> >> + if (!prop)
> >> + return false;
> >> + dev->mode_config.writeback_pixel_formats_property = prop;
> >> + }
> >> +
> >> + return true;
> >> +}
> >
> > based on a buildbot warning and the fact that the next patch starts
> > returning -ENOMEM inside the above function, I have this variant in my
> > internal tree that I was preparing to send out once I've got my i-g-t
> > tests in better shape:
> >
> > +static int create_writeback_properties(struct drm_device *dev)
> > +{
> > +   struct drm_property *prop;
> > +
> > +   if (!dev->mode_config.writeback_fb_id_property) {
> > +   prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
> > + "WRITEBACK_FB_ID",
> > + DRM_MODE_OBJECT_FB);
> > +   if (!prop)
> > +   return -ENOMEM;
> > +   dev->mode_config.writeback_fb_id_property = prop;
> > +   }
> > +
> > +   if (!dev->mode_config.writeback_pixel_formats_property) {
> > +   prop = drm_property_create(dev, DRM_MODE_PROP_BLOB | 
> > DRM_MODE_PROP_IMMUTABLE,
> > +  "WRITEBACK_PIXEL_FORMATS", 0);
> > +   if (!prop)
> > +   return -ENOMEM;
> > +   dev->mode_config.writeback_pixel_formats_property = prop;
> > +   }
> > +
> > +   return 0;
> > +}
> >
> >
> > Feel free to use this version in the next update if you're going to send
> > one, otherwise I guess we could be patching it later.
> >
> 
> hmm, I meant to keep my addition of funcs->atomic_commit() as a
> separate fixup patch so it would be easier for you to fold back into
> your patchset, but accidentally squashed it at some point and was too
> lazy to split it out again.  Sorry about that.

I'm not too fussed about who pushes Brian's framework patches into
drm-next so I don't mind at all you merging your addition. Just wanted
to make sure we're on the same page (didn't know you're going to send
your series this week).

I also feel I need to appologise for my delay in getting the i-g-t
patches into shape, I've been splitting myself between various other
tasks, not all kernel related.

Best regards,
Liviu

> 
> BR,
> -R
> 
> > Best regards,
> > Liviu
> >
> >
> >> +
> >> +static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
> >> + .destroy = drm_encoder_cleanup,
> >> +};
> >> +
> >> +/**
> >> + * drm_writeback_connector_init - Initialize a writeback connector and 
> >> its properties
> >> + * @dev: DRM device
> >> + * @wb_connector: Writeback connector to initialize
> >> + * @con_funcs: Connector funcs vtable
> >> + * @enc_helper_funcs: Encoder helper funcs vtable to be used by the 
> >> internal encoder
> >> + * @formats: Array of supported pixel formats for the writeback engine
> >> + * @n_formats: 

Re: [RFC 1/4] drm: Add writeback connector type

[Rob Clark]

On Fri, Feb 23, 2018 at 9:00 AM, Liviu Dudau  wrote:
> Hi Rob,
>
> On Fri, Feb 23, 2018 at 08:17:51AM -0500, Rob Clark wrote:
>> From: Brian Starkey 
>>
>> Writeback connectors represent writeback engines which can write the
>> CRTC output to a memory framebuffer. Add a writeback connector type and
>> related support functions.
>>
>> Drivers should initialize a writeback connector with
>> drm_writeback_connector_init() which takes care of setting up all the
>> writeback-specific details on top of the normal functionality of
>> drm_connector_init().
>>
>> Writeback connectors have a WRITEBACK_FB_ID property, used to set the
>> output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose the
>> supported writeback formats to userspace.
>>
>> When a framebuffer is attached to a writeback connector with the
>> WRITEBACK_FB_ID property, it is used only once (for the commit in which
>> it was included), and userspace can never read back the value of
>> WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
>> attached to a CRTC.
>
> [snip]
>
>> +static bool create_writeback_properties(struct drm_device *dev)
>> +{
>> + struct drm_property *prop;
>> +
>> + if (!dev->mode_config.writeback_fb_id_property) {
>> + prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
>> +   "WRITEBACK_FB_ID",
>> +   DRM_MODE_OBJECT_FB);
>> + if (!prop)
>> + return false;
>> + dev->mode_config.writeback_fb_id_property = prop;
>> + }
>> +
>> + if (!dev->mode_config.writeback_pixel_formats_property) {
>> + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB | 
>> DRM_MODE_PROP_IMMUTABLE,
>> +"WRITEBACK_PIXEL_FORMATS", 0);
>> + if (!prop)
>> + return false;
>> + dev->mode_config.writeback_pixel_formats_property = prop;
>> + }
>> +
>> + return true;
>> +}
>
> based on a buildbot warning and the fact that the next patch starts
> returning -ENOMEM inside the above function, I have this variant in my
> internal tree that I was preparing to send out once I've got my i-g-t
> tests in better shape:
>
> +static int create_writeback_properties(struct drm_device *dev)
> +{
> +   struct drm_property *prop;
> +
> +   if (!dev->mode_config.writeback_fb_id_property) {
> +   prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
> + "WRITEBACK_FB_ID",
> + DRM_MODE_OBJECT_FB);
> +   if (!prop)
> +   return -ENOMEM;
> +   dev->mode_config.writeback_fb_id_property = prop;
> +   }
> +
> +   if (!dev->mode_config.writeback_pixel_formats_property) {
> +   prop = drm_property_create(dev, DRM_MODE_PROP_BLOB | 
> DRM_MODE_PROP_IMMUTABLE,
> +  "WRITEBACK_PIXEL_FORMATS", 0);
> +   if (!prop)
> +   return -ENOMEM;
> +   dev->mode_config.writeback_pixel_formats_property = prop;
> +   }
> +
> +   return 0;
> +}
>
>
> Feel free to use this version in the next update if you're going to send
> one, otherwise I guess we could be patching it later.
>

hmm, I meant to keep my addition of funcs->atomic_commit() as a
separate fixup patch so it would be easier for you to fold back into
your patchset, but accidentally squashed it at some point and was too
lazy to split it out again.  Sorry about that.

BR,
-R

> Best regards,
> Liviu
>
>
>> +
>> +static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
>> + .destroy = drm_encoder_cleanup,
>> +};
>> +
>> +/**
>> + * drm_writeback_connector_init - Initialize a writeback connector and its 
>> properties
>> + * @dev: DRM device
>> + * @wb_connector: Writeback connector to initialize
>> + * @con_funcs: Connector funcs vtable
>> + * @enc_helper_funcs: Encoder helper funcs vtable to be used by the 
>> internal encoder
>> + * @formats: Array of supported pixel formats for the writeback engine
>> + * @n_formats: Length of the formats array
>> + *
>> + * This function creates the writeback-connector-specific properties if they
>> + * have not been already created, initializes the connector as
>> + * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
>> + * values. It will also create an internal encoder associated with the
>> + * drm_writeback_connector and set it to use the @enc_helper_funcs vtable 
>> for
>> + * the encoder helper.
>> + *
>> + * Drivers should always use this function instead of drm_connector_init() 
>> to
>> + * set up writeback connectors.
>> + *
>> + * Returns: 0 on success, or a negative error code
>> + */
>> +int drm_writeback_connector_init(struct drm_device *dev,
>> 

Re: [RFC 1/4] drm: Add writeback connector type

[Liviu Dudau]

Hi Rob,

On Fri, Feb 23, 2018 at 08:17:51AM -0500, Rob Clark wrote:
> From: Brian Starkey 
> 
> Writeback connectors represent writeback engines which can write the
> CRTC output to a memory framebuffer. Add a writeback connector type and
> related support functions.
> 
> Drivers should initialize a writeback connector with
> drm_writeback_connector_init() which takes care of setting up all the
> writeback-specific details on top of the normal functionality of
> drm_connector_init().
> 
> Writeback connectors have a WRITEBACK_FB_ID property, used to set the
> output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose the
> supported writeback formats to userspace.
> 
> When a framebuffer is attached to a writeback connector with the
> WRITEBACK_FB_ID property, it is used only once (for the commit in which
> it was included), and userspace can never read back the value of
> WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
> attached to a CRTC.

[snip]

> +static bool create_writeback_properties(struct drm_device *dev)
> +{
> + struct drm_property *prop;
> +
> + if (!dev->mode_config.writeback_fb_id_property) {
> + prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
> +   "WRITEBACK_FB_ID",
> +   DRM_MODE_OBJECT_FB);
> + if (!prop)
> + return false;
> + dev->mode_config.writeback_fb_id_property = prop;
> + }
> +
> + if (!dev->mode_config.writeback_pixel_formats_property) {
> + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB | 
> DRM_MODE_PROP_IMMUTABLE,
> +"WRITEBACK_PIXEL_FORMATS", 0);
> + if (!prop)
> + return false;
> + dev->mode_config.writeback_pixel_formats_property = prop;
> + }
> +
> + return true;
> +}

based on a buildbot warning and the fact that the next patch starts
returning -ENOMEM inside the above function, I have this variant in my
internal tree that I was preparing to send out once I've got my i-g-t
tests in better shape:

+static int create_writeback_properties(struct drm_device *dev)
+{
+   struct drm_property *prop;
+
+   if (!dev->mode_config.writeback_fb_id_property) {
+   prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
+ "WRITEBACK_FB_ID",
+ DRM_MODE_OBJECT_FB);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.writeback_fb_id_property = prop;
+   }
+
+   if (!dev->mode_config.writeback_pixel_formats_property) {
+   prop = drm_property_create(dev, DRM_MODE_PROP_BLOB | 
DRM_MODE_PROP_IMMUTABLE,
+  "WRITEBACK_PIXEL_FORMATS", 0);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.writeback_pixel_formats_property = prop;
+   }
+
+   return 0;
+}


Feel free to use this version in the next update if you're going to send
one, otherwise I guess we could be patching it later.

Best regards,
Liviu


> +
> +static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> +/**
> + * drm_writeback_connector_init - Initialize a writeback connector and its 
> properties
> + * @dev: DRM device
> + * @wb_connector: Writeback connector to initialize
> + * @con_funcs: Connector funcs vtable
> + * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal 
> encoder
> + * @formats: Array of supported pixel formats for the writeback engine
> + * @n_formats: Length of the formats array
> + *
> + * This function creates the writeback-connector-specific properties if they
> + * have not been already created, initializes the connector as
> + * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
> + * values. It will also create an internal encoder associated with the
> + * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
> + * the encoder helper.
> + *
> + * Drivers should always use this function instead of drm_connector_init() to
> + * set up writeback connectors.
> + *
> + * Returns: 0 on success, or a negative error code
> + */
> +int drm_writeback_connector_init(struct drm_device *dev,
> +  struct drm_writeback_connector *wb_connector,
> +  const struct drm_connector_funcs *con_funcs,
> +  const struct drm_encoder_helper_funcs 
> *enc_helper_funcs,
> +  const u32 *formats, int n_formats)
> +{
> + int ret;
> + struct drm_property_blob *blob;
> + struct drm_connector *connector = _connector->base;
> + struct drm_mode_config *config = >mode_config;

[RFC 1/4] drm: Add writeback connector type

[Rob Clark]

From: Brian Starkey 

Writeback connectors represent writeback engines which can write the
CRTC output to a memory framebuffer. Add a writeback connector type and
related support functions.

Drivers should initialize a writeback connector with
drm_writeback_connector_init() which takes care of setting up all the
writeback-specific details on top of the normal functionality of
drm_connector_init().

Writeback connectors have a WRITEBACK_FB_ID property, used to set the
output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose the
supported writeback formats to userspace.

When a framebuffer is attached to a writeback connector with the
WRITEBACK_FB_ID property, it is used only once (for the commit in which
it was included), and userspace can never read back the value of
WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
attached to a CRTC.

Changes since v1:
 - Added drm_writeback.c + documentation
 - Added helper to initialize writeback connector in one go
 - Added core checks
 - Squashed into a single commit
 - Dropped the client cap
 - Writeback framebuffers are no longer persistent

Changes since v2:
 Daniel Vetter:
 - Subclass drm_connector to drm_writeback_connector
 - Relax check to allow CRTC to be set without an FB
 - Add some writeback_ prefixes
 - Drop PIXEL_FORMATS_SIZE property, as it was unnecessary
 Gustavo Padovan:
 - Add drm_writeback_job to handle writeback signalling centrally

Changes since v3:
 - Rebased
 - Rename PIXEL_FORMATS -> WRITEBACK_PIXEL_FORMATS

Changes since v4:
 - Added atomic_commit() vfunc to connector helper funcs, so that
   writeback jobs are committed from atomic helpers

Signed-off-by: Brian Starkey 
[rebased and fixed conflicts]
Signed-off-by: Mihail Atanassov 
Signed-off-by: Liviu Dudau 
[rebased and added atomic_commit() vfunc for writeback jobs]
Signed-off-by: Rob Clark 
---
 Documentation/gpu/drm-kms.rst|   9 ++
 drivers/gpu/drm/Makefile |   2 +-
 drivers/gpu/drm/drm_atomic.c | 130 
 drivers/gpu/drm/drm_atomic_helper.c  |  30 
 drivers/gpu/drm/drm_connector.c  |   4 +-
 drivers/gpu/drm/drm_writeback.c  | 257 +++
 include/drm/drm_atomic.h |   3 +
 include/drm/drm_connector.h  |  13 ++
 include/drm/drm_mode_config.h|  14 ++
 include/drm/drm_modeset_helper_vtables.h |  11 ++
 include/drm/drm_writeback.h  |  89 +++
 include/uapi/drm/drm_mode.h  |   1 +
 12 files changed, 561 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_writeback.c
 create mode 100644 include/drm/drm_writeback.h

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 2dcf5b42015d..e7590dd2f71e 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -370,6 +370,15 @@ Connector Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_connector.c
:export:
 
+Writeback Connectors
+
+
+.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
+  :doc: overview
+
+.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
+  :export:
+
 Encoder Abstraction
 ===
 
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 50093ff4479b..3d708959b224 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -18,7 +18,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
-   drm_syncobj.o drm_lease.o
+   drm_syncobj.o drm_lease.o drm_writeback.o
 
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 46733d534587..019f131fe8be 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "drm_crtc_internal.h"
@@ -638,6 +639,46 @@ static void drm_atomic_crtc_print_state(struct drm_printer 
*p,
crtc->funcs->atomic_print_state(p, state);
 }
 
+/**
+ * drm_atomic_connector_check - check connector state
+ * @connector: connector to check
+ * @state: connector state to check
+ *
+ * Provides core sanity checks for connector state.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+static int drm_atomic_connector_check(struct drm_connector *connector,
+   struct drm_connector_state *state)
+{
+   struct drm_crtc_state *crtc_state;
+   struct drm_writeback_job *writeback_job = state->writeback_job;
+
+   if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) ||
+   !writeback_job)
+