diff mbox series

[V6,i-g-t,1/6] lib/igt_kms: Add writeback support

Message ID 28eefeec574b63511909dace7df3b7e6f13c3ed1.1560374714.git.rodrigosiqueiramelo@gmail.com (mailing list archive)
State New, archived
Headers show
Series igt: Add support for testing writeback connectors | expand

Commit Message

Rodrigo Siqueira June 13, 2019, 2:16 a.m. UTC
Add support in igt_kms for writeback connectors, with the ability
to attach framebuffers.

v5: Rebase and add DRM_CLIENT_CAP_WRITEBACK_CONNECTORS before
drmModeGetResources()

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
[rebased and updated to the latest igt style]
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---
 lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_kms.h |  6 ++++++
 2 files changed, 63 insertions(+)

Comments

Liviu Dudau June 13, 2019, 2:54 p.m. UTC | #1
On Wed, Jun 12, 2019 at 11:16:02PM -0300, Brian Starkey wrote:
> Add support in igt_kms for writeback connectors, with the ability
> to attach framebuffers.
> 
> v5: Rebase and add DRM_CLIENT_CAP_WRITEBACK_CONNECTORS before
> drmModeGetResources()

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Thanks for updating this! Given that I have done the last changes and this is
mostly a refresh, not sure if I should add more Reviewed-by's to the other
patches.

Best regards,
Liviu

> 
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> [rebased and updated to the latest igt style]
> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> ---
>  lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |  6 ++++++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index da188a39..140db346 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -325,6 +325,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>  	[IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
>  	[IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
>  	[IGT_CONNECTOR_VRR_CAPABLE] = "vrr_capable",
> +	[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
> +	[IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
> +	[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
>  };
>  
>  /*
> @@ -557,6 +560,7 @@ static const struct type_name connector_type_names[] = {
>  	{ DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
>  	{ DRM_MODE_CONNECTOR_DSI, "DSI" },
>  	{ DRM_MODE_CONNECTOR_DPI, "DPI" },
> +	{ DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
>  	{}
>  };
>  
> @@ -1889,6 +1893,12 @@ static void igt_output_reset(igt_output_t *output)
>  	if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
>  		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
>  					  BROADCAST_RGB_FULL);
> +	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
> +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> +	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
> +		igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> +		output->writeback_out_fence_fd = -1;
> +	}
>  }
>  
>  /**
> @@ -1901,6 +1911,8 @@ static void igt_output_reset(igt_output_t *output)
>   * For outputs:
>   * - %IGT_CONNECTOR_CRTC_ID
>   * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> + * - %IGT_CONNECTOR_WRITEBACK_FB_ID
> + * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
>   * - igt_output_override_mode() to default.
>   *
>   * For pipes:
> @@ -1970,6 +1982,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>  
>  	display->drm_fd = drm_fd;
>  
> +	drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> +
>  	resources = drmModeGetResources(display->drm_fd);
>  	if (!resources)
>  		goto out;
> @@ -2263,6 +2277,11 @@ static void igt_output_fini(igt_output_t *output)
>  	kmstest_free_connector_config(&output->config);
>  	free(output->name);
>  	output->name = NULL;
> +
> +	if (output->writeback_out_fence_fd != -1) {
> +		close(output->writeback_out_fence_fd);
> +		output->writeback_out_fence_fd = -1;
> +	}
>  }
>  
>  /**
> @@ -3325,6 +3344,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
>  					  output->props[i],
>  					  output->values[i]));
>  	}
> +
> +	if (output->writeback_out_fence_fd != -1) {
> +		close(output->writeback_out_fence_fd);
> +		output->writeback_out_fence_fd = -1;
> +	}
>  }
>  
>  /*
> @@ -3447,6 +3471,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
>  		else
>  			/* no modeset in universal commit, no change to crtc. */
>  			output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> +
> +		if (s == COMMIT_ATOMIC) {
> +			if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
> +				igt_assert(output->writeback_out_fence_fd >= 0);
> +
> +			output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
> +			output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
> +			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
> +			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> +		}
>  	}
>  
>  	if (display->first_commit) {
> @@ -4119,6 +4153,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
>  	igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
>  }
>  
> +/**
> + * igt_output_set_writeback_fb:
> + * @output: Target output
> + * @fb: Target framebuffer
> + *
> + * This function sets the given @fb to be used as the target framebuffer for the
> + * writeback engine at the next atomic commit. It will also request a writeback
> + * out fence that will contain the fd number of the out fence created by KMS if
> + * the given @fb is valid.
> + */
> +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> +{
> +	igt_display_t *display = output->display;
> +
> +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> +
> +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> +	/* only request a writeback out fence if the framebuffer is valid */
> +	if (fb)
> +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> +}
> +
>  /**
>   * igt_wait_for_vblank_count:
>   * @drm_fd: A drm file descriptor
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index a448a003..cacc6b90 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -123,6 +123,9 @@ enum igt_atomic_connector_properties {
>         IGT_CONNECTOR_BROADCAST_RGB,
>         IGT_CONNECTOR_CONTENT_PROTECTION,
>         IGT_CONNECTOR_VRR_CAPABLE,
> +       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
> +       IGT_CONNECTOR_WRITEBACK_FB_ID,
> +       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
>         IGT_NUM_CONNECTOR_PROPS
>  };
>  
> @@ -364,6 +367,8 @@ typedef struct {
>  	bool use_override_mode;
>  	drmModeModeInfo override_mode;
>  
> +	int32_t writeback_out_fence_fd;
> +
>  	/* bitmask of changed properties */
>  	uint64_t changed;
>  
> @@ -414,6 +419,7 @@ igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
>  igt_output_t *igt_output_from_connector(igt_display_t *display,
>      drmModeConnector *connector);
>  const drmModeModeInfo *igt_std_1024_mode_get(void);
> +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
>  
>  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
>  int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
> -- 
> 2.21.0
> 
> 
> -- 
> Rodrigo Siqueira
> https://siqueira.tech
Rodrigo Siqueira June 18, 2019, 9:56 p.m. UTC | #2
On Thu, Jun 13, 2019 at 11:54 AM Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>
> On Wed, Jun 12, 2019 at 11:16:02PM -0300, Brian Starkey wrote:
> > Add support in igt_kms for writeback connectors, with the ability
> > to attach framebuffers.
> >
> > v5: Rebase and add DRM_CLIENT_CAP_WRITEBACK_CONNECTORS before
> > drmModeGetResources()
>
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
>
> Thanks for updating this! Given that I have done the last changes and this is
> mostly a refresh, not sure if I should add more Reviewed-by's to the other
> patches.

Thanks Liviu!

I just forgot to add my SoB, and for some reason, gmail does not allow
me to send an email on someone behalf. Btw, I can fix it after
everybody agrees that the kms_writeback is ready for landing.


> Best regards,
> Liviu
>
> >
> > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > [rebased and updated to the latest igt style]
> > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> > ---
> >  lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_kms.h |  6 ++++++
> >  2 files changed, 63 insertions(+)
> >
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index da188a39..140db346 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -325,6 +325,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> >       [IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
> >       [IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
> >       [IGT_CONNECTOR_VRR_CAPABLE] = "vrr_capable",
> > +     [IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
> > +     [IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
> > +     [IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
> >  };
> >
> >  /*
> > @@ -557,6 +560,7 @@ static const struct type_name connector_type_names[] = {
> >       { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
> >       { DRM_MODE_CONNECTOR_DSI, "DSI" },
> >       { DRM_MODE_CONNECTOR_DPI, "DPI" },
> > +     { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
> >       {}
> >  };
> >
> > @@ -1889,6 +1893,12 @@ static void igt_output_reset(igt_output_t *output)
> >       if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
> >               igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
> >                                         BROADCAST_RGB_FULL);
> > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
> > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
> > +             igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > +             output->writeback_out_fence_fd = -1;
> > +     }
> >  }
> >
> >  /**
> > @@ -1901,6 +1911,8 @@ static void igt_output_reset(igt_output_t *output)
> >   * For outputs:
> >   * - %IGT_CONNECTOR_CRTC_ID
> >   * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> > + * - %IGT_CONNECTOR_WRITEBACK_FB_ID
> > + * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
> >   * - igt_output_override_mode() to default.
> >   *
> >   * For pipes:
> > @@ -1970,6 +1982,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> >
> >       display->drm_fd = drm_fd;
> >
> > +     drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > +
> >       resources = drmModeGetResources(display->drm_fd);
> >       if (!resources)
> >               goto out;
> > @@ -2263,6 +2277,11 @@ static void igt_output_fini(igt_output_t *output)
> >       kmstest_free_connector_config(&output->config);
> >       free(output->name);
> >       output->name = NULL;
> > +
> > +     if (output->writeback_out_fence_fd != -1) {
> > +             close(output->writeback_out_fence_fd);
> > +             output->writeback_out_fence_fd = -1;
> > +     }
> >  }
> >
> >  /**
> > @@ -3325,6 +3344,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
> >                                         output->props[i],
> >                                         output->values[i]));
> >       }
> > +
> > +     if (output->writeback_out_fence_fd != -1) {
> > +             close(output->writeback_out_fence_fd);
> > +             output->writeback_out_fence_fd = -1;
> > +     }
> >  }
> >
> >  /*
> > @@ -3447,6 +3471,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
> >               else
> >                       /* no modeset in universal commit, no change to crtc. */
> >                       output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> > +
> > +             if (s == COMMIT_ATOMIC) {
> > +                     if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
> > +                             igt_assert(output->writeback_out_fence_fd >= 0);
> > +
> > +                     output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
> > +                     output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
> > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
> > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > +             }
> >       }
> >
> >       if (display->first_commit) {
> > @@ -4119,6 +4153,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
> >       igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
> >  }
> >
> > +/**
> > + * igt_output_set_writeback_fb:
> > + * @output: Target output
> > + * @fb: Target framebuffer
> > + *
> > + * This function sets the given @fb to be used as the target framebuffer for the
> > + * writeback engine at the next atomic commit. It will also request a writeback
> > + * out fence that will contain the fd number of the out fence created by KMS if
> > + * the given @fb is valid.
> > + */
> > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > +{
> > +     igt_display_t *display = output->display;
> > +
> > +     LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > +
> > +     igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > +     /* only request a writeback out fence if the framebuffer is valid */
> > +     if (fb)
> > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > +                                       (ptrdiff_t)&output->writeback_out_fence_fd);
> > +}
> > +
> >  /**
> >   * igt_wait_for_vblank_count:
> >   * @drm_fd: A drm file descriptor
> > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > index a448a003..cacc6b90 100644
> > --- a/lib/igt_kms.h
> > +++ b/lib/igt_kms.h
> > @@ -123,6 +123,9 @@ enum igt_atomic_connector_properties {
> >         IGT_CONNECTOR_BROADCAST_RGB,
> >         IGT_CONNECTOR_CONTENT_PROTECTION,
> >         IGT_CONNECTOR_VRR_CAPABLE,
> > +       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
> > +       IGT_CONNECTOR_WRITEBACK_FB_ID,
> > +       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> >         IGT_NUM_CONNECTOR_PROPS
> >  };
> >
> > @@ -364,6 +367,8 @@ typedef struct {
> >       bool use_override_mode;
> >       drmModeModeInfo override_mode;
> >
> > +     int32_t writeback_out_fence_fd;
> > +
> >       /* bitmask of changed properties */
> >       uint64_t changed;
> >
> > @@ -414,6 +419,7 @@ igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
> >  igt_output_t *igt_output_from_connector(igt_display_t *display,
> >      drmModeConnector *connector);
> >  const drmModeModeInfo *igt_std_1024_mode_get(void);
> > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
> >
> >  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
> >  int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
> > --
> > 2.21.0
> >
> >
> > --
> > Rodrigo Siqueira
> > https://siqueira.tech
>
>
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
Ser, Simon July 3, 2019, 12:15 p.m. UTC | #3
On Tue, 2019-06-18 at 18:56 -0300, Rodrigo Siqueira wrote:
> On Thu, Jun 13, 2019 at 11:54 AM Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > On Wed, Jun 12, 2019 at 11:16:02PM -0300, Brian Starkey wrote:
> > > Add support in igt_kms for writeback connectors, with the ability
> > > to attach framebuffers.
> > > 
> > > v5: Rebase and add DRM_CLIENT_CAP_WRITEBACK_CONNECTORS before
> > > drmModeGetResources()
> > 
> > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > 
> > Thanks for updating this! Given that I have done the last changes and this is
> > mostly a refresh, not sure if I should add more Reviewed-by's to the other
> > patches.
> 
> Thanks Liviu!
> 
> I just forgot to add my SoB, and for some reason, gmail does not allow
> me to send an email on someone behalf.

FWIW, that's a good thing, and it's required to pass DMARC checks.

Instead, git-send-email should add a From line at the beginning of the
message when sending a patch on behalf of someone else. I wonder what
happened here.

> Btw, I can fix it after
> everybody agrees that the kms_writeback is ready for landing.
> 
> 
> > Best regards,
> > Liviu
> > 
> > > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > > [rebased and updated to the latest igt style]
> > > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> > > ---
> > >  lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/igt_kms.h |  6 ++++++
> > >  2 files changed, 63 insertions(+)
> > > 
> > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > index da188a39..140db346 100644
> > > --- a/lib/igt_kms.c
> > > +++ b/lib/igt_kms.c
> > > @@ -325,6 +325,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> > >       [IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
> > >       [IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
> > >       [IGT_CONNECTOR_VRR_CAPABLE] = "vrr_capable",
> > > +     [IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
> > > +     [IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
> > > +     [IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
> > >  };
> > > 
> > >  /*
> > > @@ -557,6 +560,7 @@ static const struct type_name connector_type_names[] = {
> > >       { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
> > >       { DRM_MODE_CONNECTOR_DSI, "DSI" },
> > >       { DRM_MODE_CONNECTOR_DPI, "DPI" },
> > > +     { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
> > >       {}
> > >  };
> > > 
> > > @@ -1889,6 +1893,12 @@ static void igt_output_reset(igt_output_t *output)
> > >       if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
> > >               igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
> > >                                         BROADCAST_RGB_FULL);
> > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
> > > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
> > > +             igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > +             output->writeback_out_fence_fd = -1;
> > > +     }
> > >  }
> > > 
> > >  /**
> > > @@ -1901,6 +1911,8 @@ static void igt_output_reset(igt_output_t *output)
> > >   * For outputs:
> > >   * - %IGT_CONNECTOR_CRTC_ID
> > >   * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> > > + * - %IGT_CONNECTOR_WRITEBACK_FB_ID
> > > + * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
> > >   * - igt_output_override_mode() to default.
> > >   *
> > >   * For pipes:
> > > @@ -1970,6 +1982,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> > > 
> > >       display->drm_fd = drm_fd;
> > > 
> > > +     drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > > +
> > >       resources = drmModeGetResources(display->drm_fd);
> > >       if (!resources)
> > >               goto out;
> > > @@ -2263,6 +2277,11 @@ static void igt_output_fini(igt_output_t *output)
> > >       kmstest_free_connector_config(&output->config);
> > >       free(output->name);
> > >       output->name = NULL;
> > > +
> > > +     if (output->writeback_out_fence_fd != -1) {
> > > +             close(output->writeback_out_fence_fd);
> > > +             output->writeback_out_fence_fd = -1;
> > > +     }
> > >  }
> > > 
> > >  /**
> > > @@ -3325,6 +3344,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
> > >                                         output->props[i],
> > >                                         output->values[i]));
> > >       }
> > > +
> > > +     if (output->writeback_out_fence_fd != -1) {
> > > +             close(output->writeback_out_fence_fd);
> > > +             output->writeback_out_fence_fd = -1;
> > > +     }
> > >  }
> > > 
> > >  /*
> > > @@ -3447,6 +3471,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
> > >               else
> > >                       /* no modeset in universal commit, no change to crtc. */
> > >                       output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> > > +
> > > +             if (s == COMMIT_ATOMIC) {
> > > +                     if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
> > > +                             igt_assert(output->writeback_out_fence_fd >= 0);
> > > +
> > > +                     output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
> > > +                     output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
> > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
> > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > +             }
> > >       }
> > > 
> > >       if (display->first_commit) {
> > > @@ -4119,6 +4153,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
> > >       igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
> > >  }
> > > 
> > > +/**
> > > + * igt_output_set_writeback_fb:
> > > + * @output: Target output
> > > + * @fb: Target framebuffer
> > > + *
> > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > + * the given @fb is valid.
> > > + */
> > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > +{
> > > +     igt_display_t *display = output->display;
> > > +
> > > +     LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > +
> > > +     igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > +     /* only request a writeback out fence if the framebuffer is valid */
> > > +     if (fb)
> > > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > +                                       (ptrdiff_t)&output->writeback_out_fence_fd);
> > > +}
> > > +
> > >  /**
> > >   * igt_wait_for_vblank_count:
> > >   * @drm_fd: A drm file descriptor
> > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > index a448a003..cacc6b90 100644
> > > --- a/lib/igt_kms.h
> > > +++ b/lib/igt_kms.h
> > > @@ -123,6 +123,9 @@ enum igt_atomic_connector_properties {
> > >         IGT_CONNECTOR_BROADCAST_RGB,
> > >         IGT_CONNECTOR_CONTENT_PROTECTION,
> > >         IGT_CONNECTOR_VRR_CAPABLE,
> > > +       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
> > > +       IGT_CONNECTOR_WRITEBACK_FB_ID,
> > > +       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > >         IGT_NUM_CONNECTOR_PROPS
> > >  };
> > > 
> > > @@ -364,6 +367,8 @@ typedef struct {
> > >       bool use_override_mode;
> > >       drmModeModeInfo override_mode;
> > > 
> > > +     int32_t writeback_out_fence_fd;
> > > +
> > >       /* bitmask of changed properties */
> > >       uint64_t changed;
> > > 
> > > @@ -414,6 +419,7 @@ igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
> > >  igt_output_t *igt_output_from_connector(igt_display_t *display,
> > >      drmModeConnector *connector);
> > >  const drmModeModeInfo *igt_std_1024_mode_get(void);
> > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
> > > 
> > >  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
> > >  int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
> > > --
> > > 2.21.0
> > > 
> > > 
> > > --
> > > Rodrigo Siqueira
> > > https://siqueira.tech
> > 
> > 
> > --
> > ====================
> > > I would like to |
> > > fix the world,  |
> > > but they're not |
> > > giving me the   |
> >  \ source code!  /
> >   ---------------
> >     ¯\_(ツ)_/¯
> 
>
Rodrigo Siqueira July 9, 2019, 2:32 p.m. UTC | #4
On Wed, Jul 3, 2019 at 9:15 AM Ser, Simon <simon.ser@intel.com> wrote:
>
> On Tue, 2019-06-18 at 18:56 -0300, Rodrigo Siqueira wrote:
> > On Thu, Jun 13, 2019 at 11:54 AM Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > > On Wed, Jun 12, 2019 at 11:16:02PM -0300, Brian Starkey wrote:
> > > > Add support in igt_kms for writeback connectors, with the ability
> > > > to attach framebuffers.
> > > >
> > > > v5: Rebase and add DRM_CLIENT_CAP_WRITEBACK_CONNECTORS before
> > > > drmModeGetResources()
> > >
> > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > >
> > > Thanks for updating this! Given that I have done the last changes and this is
> > > mostly a refresh, not sure if I should add more Reviewed-by's to the other
> > > patches.
> >
> > Thanks Liviu!
> >
> > I just forgot to add my SoB, and for some reason, gmail does not allow
> > me to send an email on someone behalf.
>
> FWIW, that's a good thing, and it's required to pass DMARC checks.
>
> Instead, git-send-email should add a From line at the beginning of the
> message when sending a patch on behalf of someone else. I wonder what
> happened here.

Thank you for your help.

I’m using neomutt for sending patches, and I’ll take a look at
git-send-email. Additionally, it looks like Gmail requires that I add
a new account in order to allow me to send patches on someone behalf.
I’ll take some time to read about this issue, and I will try to resend
the patchset again.

Btw, if you have time, could you take a look in this series?

Best regards

> > Btw, I can fix it after
> > everybody agrees that the kms_writeback is ready for landing.
> >
> >
> > > Best regards,
> > > Liviu
> > >
> > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > > > [rebased and updated to the latest igt style]
> > > > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > ---
> > > >  lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  lib/igt_kms.h |  6 ++++++
> > > >  2 files changed, 63 insertions(+)
> > > >
> > > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > > index da188a39..140db346 100644
> > > > --- a/lib/igt_kms.c
> > > > +++ b/lib/igt_kms.c
> > > > @@ -325,6 +325,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> > > >       [IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
> > > >       [IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
> > > >       [IGT_CONNECTOR_VRR_CAPABLE] = "vrr_capable",
> > > > +     [IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
> > > > +     [IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
> > > > +     [IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
> > > >  };
> > > >
> > > >  /*
> > > > @@ -557,6 +560,7 @@ static const struct type_name connector_type_names[] = {
> > > >       { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
> > > >       { DRM_MODE_CONNECTOR_DSI, "DSI" },
> > > >       { DRM_MODE_CONNECTOR_DPI, "DPI" },
> > > > +     { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
> > > >       {}
> > > >  };
> > > >
> > > > @@ -1889,6 +1893,12 @@ static void igt_output_reset(igt_output_t *output)
> > > >       if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
> > > >               igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
> > > >                                         BROADCAST_RGB_FULL);
> > > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
> > > > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> > > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
> > > > +             igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > > +             output->writeback_out_fence_fd = -1;
> > > > +     }
> > > >  }
> > > >
> > > >  /**
> > > > @@ -1901,6 +1911,8 @@ static void igt_output_reset(igt_output_t *output)
> > > >   * For outputs:
> > > >   * - %IGT_CONNECTOR_CRTC_ID
> > > >   * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> > > > + * - %IGT_CONNECTOR_WRITEBACK_FB_ID
> > > > + * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
> > > >   * - igt_output_override_mode() to default.
> > > >   *
> > > >   * For pipes:
> > > > @@ -1970,6 +1982,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> > > >
> > > >       display->drm_fd = drm_fd;
> > > >
> > > > +     drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > > > +
> > > >       resources = drmModeGetResources(display->drm_fd);
> > > >       if (!resources)
> > > >               goto out;
> > > > @@ -2263,6 +2277,11 @@ static void igt_output_fini(igt_output_t *output)
> > > >       kmstest_free_connector_config(&output->config);
> > > >       free(output->name);
> > > >       output->name = NULL;
> > > > +
> > > > +     if (output->writeback_out_fence_fd != -1) {
> > > > +             close(output->writeback_out_fence_fd);
> > > > +             output->writeback_out_fence_fd = -1;
> > > > +     }
> > > >  }
> > > >
> > > >  /**
> > > > @@ -3325,6 +3344,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
> > > >                                         output->props[i],
> > > >                                         output->values[i]));
> > > >       }
> > > > +
> > > > +     if (output->writeback_out_fence_fd != -1) {
> > > > +             close(output->writeback_out_fence_fd);
> > > > +             output->writeback_out_fence_fd = -1;
> > > > +     }
> > > >  }
> > > >
> > > >  /*
> > > > @@ -3447,6 +3471,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
> > > >               else
> > > >                       /* no modeset in universal commit, no change to crtc. */
> > > >                       output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> > > > +
> > > > +             if (s == COMMIT_ATOMIC) {
> > > > +                     if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
> > > > +                             igt_assert(output->writeback_out_fence_fd >= 0);
> > > > +
> > > > +                     output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
> > > > +                     output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
> > > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
> > > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > > +             }
> > > >       }
> > > >
> > > >       if (display->first_commit) {
> > > > @@ -4119,6 +4153,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
> > > >       igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
> > > >  }
> > > >
> > > > +/**
> > > > + * igt_output_set_writeback_fb:
> > > > + * @output: Target output
> > > > + * @fb: Target framebuffer
> > > > + *
> > > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > > + * the given @fb is valid.
> > > > + */
> > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > > +{
> > > > +     igt_display_t *display = output->display;
> > > > +
> > > > +     LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > > +
> > > > +     igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > > +     /* only request a writeback out fence if the framebuffer is valid */
> > > > +     if (fb)
> > > > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > +                                       (ptrdiff_t)&output->writeback_out_fence_fd);
> > > > +}
> > > > +
> > > >  /**
> > > >   * igt_wait_for_vblank_count:
> > > >   * @drm_fd: A drm file descriptor
> > > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > > index a448a003..cacc6b90 100644
> > > > --- a/lib/igt_kms.h
> > > > +++ b/lib/igt_kms.h
> > > > @@ -123,6 +123,9 @@ enum igt_atomic_connector_properties {
> > > >         IGT_CONNECTOR_BROADCAST_RGB,
> > > >         IGT_CONNECTOR_CONTENT_PROTECTION,
> > > >         IGT_CONNECTOR_VRR_CAPABLE,
> > > > +       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
> > > > +       IGT_CONNECTOR_WRITEBACK_FB_ID,
> > > > +       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > >         IGT_NUM_CONNECTOR_PROPS
> > > >  };
> > > >
> > > > @@ -364,6 +367,8 @@ typedef struct {
> > > >       bool use_override_mode;
> > > >       drmModeModeInfo override_mode;
> > > >
> > > > +     int32_t writeback_out_fence_fd;
> > > > +
> > > >       /* bitmask of changed properties */
> > > >       uint64_t changed;
> > > >
> > > > @@ -414,6 +419,7 @@ igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
> > > >  igt_output_t *igt_output_from_connector(igt_display_t *display,
> > > >      drmModeConnector *connector);
> > > >  const drmModeModeInfo *igt_std_1024_mode_get(void);
> > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
> > > >
> > > >  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
> > > >  int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
> > > > --
> > > > 2.21.0
> > > >
> > > >
> > > > --
> > > > Rodrigo Siqueira
> > > > https://siqueira.tech
> > >
> > >
> > > --
> > > ====================
> > > > I would like to |
> > > > fix the world,  |
> > > > but they're not |
> > > > giving me the   |
> > >  \ source code!  /
> > >   ---------------
> > >     ¯\_(ツ)_/¯
> >
> >
Ser, Simon July 9, 2019, 2:42 p.m. UTC | #5
On Tue, 2019-07-09 at 11:32 -0300, Rodrigo Siqueira wrote:
> On Wed, Jul 3, 2019 at 9:15 AM Ser, Simon <simon.ser@intel.com> wrote:
> > On Tue, 2019-06-18 at 18:56 -0300, Rodrigo Siqueira wrote:
> > > On Thu, Jun 13, 2019 at 11:54 AM Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > > > On Wed, Jun 12, 2019 at 11:16:02PM -0300, Brian Starkey wrote:
> > > > > Add support in igt_kms for writeback connectors, with the ability
> > > > > to attach framebuffers.
> > > > > 
> > > > > v5: Rebase and add DRM_CLIENT_CAP_WRITEBACK_CONNECTORS before
> > > > > drmModeGetResources()
> > > > 
> > > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > 
> > > > Thanks for updating this! Given that I have done the last changes and this is
> > > > mostly a refresh, not sure if I should add more Reviewed-by's to the other
> > > > patches.
> > > 
> > > Thanks Liviu!
> > > 
> > > I just forgot to add my SoB, and for some reason, gmail does not allow
> > > me to send an email on someone behalf.
> > 
> > FWIW, that's a good thing, and it's required to pass DMARC checks.
> > 
> > Instead, git-send-email should add a From line at the beginning of the
> > message when sending a patch on behalf of someone else. I wonder what
> > happened here.
> 
> Thank you for your help.
> 
> I’m using neomutt for sending patches, and I’ll take a look at
> git-send-email. Additionally, it looks like Gmail requires that I add
> a new account in order to allow me to send patches on someone behalf.
> I’ll take some time to read about this issue, and I will try to resend
> the patchset again.

When sending a patch on someone else's behalf, you shouldn't send the
e-mail with the sender (From header) set to someone else. You should
use your normal address. The From line in the body of the message will
make Git understand the author is someone else.

So no new account setup required.

I really recommend using git-send-email for patches. There are too many
ways to make a mistake if you try sending emails manually. Here is a
tutorial:
https://git-send-email.io/

> Btw, if you have time, could you take a look in this series?

Sure, I've already began looking at the series. I'll continue soon.

Thanks for your work!

> Best regards
> 
> > > Btw, I can fix it after
> > > everybody agrees that the kms_writeback is ready for landing.
> > > 
> > > 
> > > > Best regards,
> > > > Liviu
> > > > 
> > > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > > > > [rebased and updated to the latest igt style]
> > > > > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > > ---
> > > > >  lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  lib/igt_kms.h |  6 ++++++
> > > > >  2 files changed, 63 insertions(+)
> > > > > 
> > > > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > > > index da188a39..140db346 100644
> > > > > --- a/lib/igt_kms.c
> > > > > +++ b/lib/igt_kms.c
> > > > > @@ -325,6 +325,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> > > > >       [IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
> > > > >       [IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
> > > > >       [IGT_CONNECTOR_VRR_CAPABLE] = "vrr_capable",
> > > > > +     [IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
> > > > > +     [IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
> > > > > +     [IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
> > > > >  };
> > > > > 
> > > > >  /*
> > > > > @@ -557,6 +560,7 @@ static const struct type_name connector_type_names[] = {
> > > > >       { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
> > > > >       { DRM_MODE_CONNECTOR_DSI, "DSI" },
> > > > >       { DRM_MODE_CONNECTOR_DPI, "DPI" },
> > > > > +     { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
> > > > >       {}
> > > > >  };
> > > > > 
> > > > > @@ -1889,6 +1893,12 @@ static void igt_output_reset(igt_output_t *output)
> > > > >       if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
> > > > >               igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
> > > > >                                         BROADCAST_RGB_FULL);
> > > > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
> > > > > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> > > > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
> > > > > +             igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > > > +             output->writeback_out_fence_fd = -1;
> > > > > +     }
> > > > >  }
> > > > > 
> > > > >  /**
> > > > > @@ -1901,6 +1911,8 @@ static void igt_output_reset(igt_output_t *output)
> > > > >   * For outputs:
> > > > >   * - %IGT_CONNECTOR_CRTC_ID
> > > > >   * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> > > > > + * - %IGT_CONNECTOR_WRITEBACK_FB_ID
> > > > > + * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
> > > > >   * - igt_output_override_mode() to default.
> > > > >   *
> > > > >   * For pipes:
> > > > > @@ -1970,6 +1982,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> > > > > 
> > > > >       display->drm_fd = drm_fd;
> > > > > 
> > > > > +     drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > > > > +
> > > > >       resources = drmModeGetResources(display->drm_fd);
> > > > >       if (!resources)
> > > > >               goto out;
> > > > > @@ -2263,6 +2277,11 @@ static void igt_output_fini(igt_output_t *output)
> > > > >       kmstest_free_connector_config(&output->config);
> > > > >       free(output->name);
> > > > >       output->name = NULL;
> > > > > +
> > > > > +     if (output->writeback_out_fence_fd != -1) {
> > > > > +             close(output->writeback_out_fence_fd);
> > > > > +             output->writeback_out_fence_fd = -1;
> > > > > +     }
> > > > >  }
> > > > > 
> > > > >  /**
> > > > > @@ -3325,6 +3344,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
> > > > >                                         output->props[i],
> > > > >                                         output->values[i]));
> > > > >       }
> > > > > +
> > > > > +     if (output->writeback_out_fence_fd != -1) {
> > > > > +             close(output->writeback_out_fence_fd);
> > > > > +             output->writeback_out_fence_fd = -1;
> > > > > +     }
> > > > >  }
> > > > > 
> > > > >  /*
> > > > > @@ -3447,6 +3471,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
> > > > >               else
> > > > >                       /* no modeset in universal commit, no change to crtc. */
> > > > >                       output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> > > > > +
> > > > > +             if (s == COMMIT_ATOMIC) {
> > > > > +                     if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
> > > > > +                             igt_assert(output->writeback_out_fence_fd >= 0);
> > > > > +
> > > > > +                     output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
> > > > > +                     output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
> > > > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
> > > > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > > > +             }
> > > > >       }
> > > > > 
> > > > >       if (display->first_commit) {
> > > > > @@ -4119,6 +4153,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
> > > > >       igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
> > > > >  }
> > > > > 
> > > > > +/**
> > > > > + * igt_output_set_writeback_fb:
> > > > > + * @output: Target output
> > > > > + * @fb: Target framebuffer
> > > > > + *
> > > > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > > > + * the given @fb is valid.
> > > > > + */
> > > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > > > +{
> > > > > +     igt_display_t *display = output->display;
> > > > > +
> > > > > +     LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > > > +
> > > > > +     igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > > > +     /* only request a writeback out fence if the framebuffer is valid */
> > > > > +     if (fb)
> > > > > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > > +                                       (ptrdiff_t)&output->writeback_out_fence_fd);
> > > > > +}
> > > > > +
> > > > >  /**
> > > > >   * igt_wait_for_vblank_count:
> > > > >   * @drm_fd: A drm file descriptor
> > > > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > > > index a448a003..cacc6b90 100644
> > > > > --- a/lib/igt_kms.h
> > > > > +++ b/lib/igt_kms.h
> > > > > @@ -123,6 +123,9 @@ enum igt_atomic_connector_properties {
> > > > >         IGT_CONNECTOR_BROADCAST_RGB,
> > > > >         IGT_CONNECTOR_CONTENT_PROTECTION,
> > > > >         IGT_CONNECTOR_VRR_CAPABLE,
> > > > > +       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
> > > > > +       IGT_CONNECTOR_WRITEBACK_FB_ID,
> > > > > +       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > >         IGT_NUM_CONNECTOR_PROPS
> > > > >  };
> > > > > 
> > > > > @@ -364,6 +367,8 @@ typedef struct {
> > > > >       bool use_override_mode;
> > > > >       drmModeModeInfo override_mode;
> > > > > 
> > > > > +     int32_t writeback_out_fence_fd;
> > > > > +
> > > > >       /* bitmask of changed properties */
> > > > >       uint64_t changed;
> > > > > 
> > > > > @@ -414,6 +419,7 @@ igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
> > > > >  igt_output_t *igt_output_from_connector(igt_display_t *display,
> > > > >      drmModeConnector *connector);
> > > > >  const drmModeModeInfo *igt_std_1024_mode_get(void);
> > > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
> > > > > 
> > > > >  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
> > > > >  int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
> > > > > --
> > > > > 2.21.0
> > > > > 
> > > > > 
> > > > > --
> > > > > Rodrigo Siqueira
> > > > > https://siqueira.tech
> > > > 
> > > > --
> > > > ====================
> > > > > I would like to |
> > > > > fix the world,  |
> > > > > but they're not |
> > > > > giving me the   |
> > > >  \ source code!  /
> > > >   ---------------
> > > >     ¯\_(ツ)_/¯
> 
>
Rodrigo Siqueira July 9, 2019, 3:07 p.m. UTC | #6
On Tue, Jul 9, 2019 at 11:42 AM Ser, Simon <simon.ser@intel.com> wrote:
>
> On Tue, 2019-07-09 at 11:32 -0300, Rodrigo Siqueira wrote:
> > On Wed, Jul 3, 2019 at 9:15 AM Ser, Simon <simon.ser@intel.com> wrote:
> > > On Tue, 2019-06-18 at 18:56 -0300, Rodrigo Siqueira wrote:
> > > > On Thu, Jun 13, 2019 at 11:54 AM Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > > > > On Wed, Jun 12, 2019 at 11:16:02PM -0300, Brian Starkey wrote:
> > > > > > Add support in igt_kms for writeback connectors, with the ability
> > > > > > to attach framebuffers.
> > > > > >
> > > > > > v5: Rebase and add DRM_CLIENT_CAP_WRITEBACK_CONNECTORS before
> > > > > > drmModeGetResources()
> > > > >
> > > > > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > >
> > > > > Thanks for updating this! Given that I have done the last changes and this is
> > > > > mostly a refresh, not sure if I should add more Reviewed-by's to the other
> > > > > patches.
> > > >
> > > > Thanks Liviu!
> > > >
> > > > I just forgot to add my SoB, and for some reason, gmail does not allow
> > > > me to send an email on someone behalf.
> > >
> > > FWIW, that's a good thing, and it's required to pass DMARC checks.
> > >
> > > Instead, git-send-email should add a From line at the beginning of the
> > > message when sending a patch on behalf of someone else. I wonder what
> > > happened here.
> >
> > Thank you for your help.
> >
> > I’m using neomutt for sending patches, and I’ll take a look at
> > git-send-email. Additionally, it looks like Gmail requires that I add
> > a new account in order to allow me to send patches on someone behalf.
> > I’ll take some time to read about this issue, and I will try to resend
> > the patchset again.
>
> When sending a patch on someone else's behalf, you shouldn't send the
> e-mail with the sender (From header) set to someone else. You should
> use your normal address. The From line in the body of the message will
> make Git understand the author is someone else.
>
> So no new account setup required.
>
> I really recommend using git-send-email for patches. There are too many
> ways to make a mistake if you try sending emails manually. Here is a
> tutorial:
> https://git-send-email.io/

Thank you very much for the tutorial! I'll read it and prepare to
migrate from 'neomutt -H' to git-send-mail.

> > Btw, if you have time, could you take a look in this series?
>
> Sure, I've already began looking at the series. I'll continue soon.

Thank you again for your help.

> Thanks for your work!
>
> > Best regards
> >
> > > > Btw, I can fix it after
> > > > everybody agrees that the kms_writeback is ready for landing.
> > > >
> > > >
> > > > > Best regards,
> > > > > Liviu
> > > > >
> > > > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > > > > > [rebased and updated to the latest igt style]
> > > > > > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > > > ---
> > > > > >  lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  lib/igt_kms.h |  6 ++++++
> > > > > >  2 files changed, 63 insertions(+)
> > > > > >
> > > > > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > > > > index da188a39..140db346 100644
> > > > > > --- a/lib/igt_kms.c
> > > > > > +++ b/lib/igt_kms.c
> > > > > > @@ -325,6 +325,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> > > > > >       [IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
> > > > > >       [IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
> > > > > >       [IGT_CONNECTOR_VRR_CAPABLE] = "vrr_capable",
> > > > > > +     [IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
> > > > > > +     [IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
> > > > > > +     [IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
> > > > > >  };
> > > > > >
> > > > > >  /*
> > > > > > @@ -557,6 +560,7 @@ static const struct type_name connector_type_names[] = {
> > > > > >       { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
> > > > > >       { DRM_MODE_CONNECTOR_DSI, "DSI" },
> > > > > >       { DRM_MODE_CONNECTOR_DPI, "DPI" },
> > > > > > +     { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
> > > > > >       {}
> > > > > >  };
> > > > > >
> > > > > > @@ -1889,6 +1893,12 @@ static void igt_output_reset(igt_output_t *output)
> > > > > >       if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
> > > > > >               igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
> > > > > >                                         BROADCAST_RGB_FULL);
> > > > > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
> > > > > > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> > > > > > +     if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
> > > > > > +             igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > > > > +             output->writeback_out_fence_fd = -1;
> > > > > > +     }
> > > > > >  }
> > > > > >
> > > > > >  /**
> > > > > > @@ -1901,6 +1911,8 @@ static void igt_output_reset(igt_output_t *output)
> > > > > >   * For outputs:
> > > > > >   * - %IGT_CONNECTOR_CRTC_ID
> > > > > >   * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> > > > > > + * - %IGT_CONNECTOR_WRITEBACK_FB_ID
> > > > > > + * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
> > > > > >   * - igt_output_override_mode() to default.
> > > > > >   *
> > > > > >   * For pipes:
> > > > > > @@ -1970,6 +1982,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> > > > > >
> > > > > >       display->drm_fd = drm_fd;
> > > > > >
> > > > > > +     drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> > > > > > +
> > > > > >       resources = drmModeGetResources(display->drm_fd);
> > > > > >       if (!resources)
> > > > > >               goto out;
> > > > > > @@ -2263,6 +2277,11 @@ static void igt_output_fini(igt_output_t *output)
> > > > > >       kmstest_free_connector_config(&output->config);
> > > > > >       free(output->name);
> > > > > >       output->name = NULL;
> > > > > > +
> > > > > > +     if (output->writeback_out_fence_fd != -1) {
> > > > > > +             close(output->writeback_out_fence_fd);
> > > > > > +             output->writeback_out_fence_fd = -1;
> > > > > > +     }
> > > > > >  }
> > > > > >
> > > > > >  /**
> > > > > > @@ -3325,6 +3344,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
> > > > > >                                         output->props[i],
> > > > > >                                         output->values[i]));
> > > > > >       }
> > > > > > +
> > > > > > +     if (output->writeback_out_fence_fd != -1) {
> > > > > > +             close(output->writeback_out_fence_fd);
> > > > > > +             output->writeback_out_fence_fd = -1;
> > > > > > +     }
> > > > > >  }
> > > > > >
> > > > > >  /*
> > > > > > @@ -3447,6 +3471,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
> > > > > >               else
> > > > > >                       /* no modeset in universal commit, no change to crtc. */
> > > > > >                       output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> > > > > > +
> > > > > > +             if (s == COMMIT_ATOMIC) {
> > > > > > +                     if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
> > > > > > +                             igt_assert(output->writeback_out_fence_fd >= 0);
> > > > > > +
> > > > > > +                     output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
> > > > > > +                     output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
> > > > > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
> > > > > > +                     igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> > > > > > +             }
> > > > > >       }
> > > > > >
> > > > > >       if (display->first_commit) {
> > > > > > @@ -4119,6 +4153,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
> > > > > >       igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
> > > > > >  }
> > > > > >
> > > > > > +/**
> > > > > > + * igt_output_set_writeback_fb:
> > > > > > + * @output: Target output
> > > > > > + * @fb: Target framebuffer
> > > > > > + *
> > > > > > + * This function sets the given @fb to be used as the target framebuffer for the
> > > > > > + * writeback engine at the next atomic commit. It will also request a writeback
> > > > > > + * out fence that will contain the fd number of the out fence created by KMS if
> > > > > > + * the given @fb is valid.
> > > > > > + */
> > > > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> > > > > > +{
> > > > > > +     igt_display_t *display = output->display;
> > > > > > +
> > > > > > +     LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> > > > > > +
> > > > > > +     igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> > > > > > +     /* only request a writeback out fence if the framebuffer is valid */
> > > > > > +     if (fb)
> > > > > > +             igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > > > +                                       (ptrdiff_t)&output->writeback_out_fence_fd);
> > > > > > +}
> > > > > > +
> > > > > >  /**
> > > > > >   * igt_wait_for_vblank_count:
> > > > > >   * @drm_fd: A drm file descriptor
> > > > > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > > > > index a448a003..cacc6b90 100644
> > > > > > --- a/lib/igt_kms.h
> > > > > > +++ b/lib/igt_kms.h
> > > > > > @@ -123,6 +123,9 @@ enum igt_atomic_connector_properties {
> > > > > >         IGT_CONNECTOR_BROADCAST_RGB,
> > > > > >         IGT_CONNECTOR_CONTENT_PROTECTION,
> > > > > >         IGT_CONNECTOR_VRR_CAPABLE,
> > > > > > +       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
> > > > > > +       IGT_CONNECTOR_WRITEBACK_FB_ID,
> > > > > > +       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> > > > > >         IGT_NUM_CONNECTOR_PROPS
> > > > > >  };
> > > > > >
> > > > > > @@ -364,6 +367,8 @@ typedef struct {
> > > > > >       bool use_override_mode;
> > > > > >       drmModeModeInfo override_mode;
> > > > > >
> > > > > > +     int32_t writeback_out_fence_fd;
> > > > > > +
> > > > > >       /* bitmask of changed properties */
> > > > > >       uint64_t changed;
> > > > > >
> > > > > > @@ -414,6 +419,7 @@ igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
> > > > > >  igt_output_t *igt_output_from_connector(igt_display_t *display,
> > > > > >      drmModeConnector *connector);
> > > > > >  const drmModeModeInfo *igt_std_1024_mode_get(void);
> > > > > > +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
> > > > > >
> > > > > >  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
> > > > > >  int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
> > > > > > --
> > > > > > 2.21.0
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Rodrigo Siqueira
> > > > > > https://siqueira.tech
> > > > >
> > > > > --
> > > > > ====================
> > > > > > I would like to |
> > > > > > fix the world,  |
> > > > > > but they're not |
> > > > > > giving me the   |
> > > > >  \ source code!  /
> > > > >   ---------------
> > > > >     ¯\_(ツ)_/¯
> >
> >
Ser, Simon July 10, 2019, 8:48 a.m. UTC | #7
On Wed, 2019-06-12 at 23:16 -0300, Brian Starkey wrote:
> Add support in igt_kms for writeback connectors, with the ability
> to attach framebuffers.
> 
> v5: Rebase and add DRM_CLIENT_CAP_WRITEBACK_CONNECTORS before
> drmModeGetResources()
> 
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> [rebased and updated to the latest igt style]
> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>

This patch LGTM.

Reviewed-by: Simon Ser <simon.ser@intel.com>

> ---
>  lib/igt_kms.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |  6 ++++++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index da188a39..140db346 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -325,6 +325,9 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>  	[IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
>  	[IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
>  	[IGT_CONNECTOR_VRR_CAPABLE] = "vrr_capable",
> +	[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
> +	[IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
> +	[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
>  };
>  
>  /*
> @@ -557,6 +560,7 @@ static const struct type_name connector_type_names[] = {
>  	{ DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
>  	{ DRM_MODE_CONNECTOR_DSI, "DSI" },
>  	{ DRM_MODE_CONNECTOR_DPI, "DPI" },
> +	{ DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
>  	{}
>  };
>  
> @@ -1889,6 +1893,12 @@ static void igt_output_reset(igt_output_t *output)
>  	if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
>  		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
>  					  BROADCAST_RGB_FULL);
> +	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
> +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> +	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
> +		igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> +		output->writeback_out_fence_fd = -1;
> +	}
>  }
>  
>  /**
> @@ -1901,6 +1911,8 @@ static void igt_output_reset(igt_output_t *output)
>   * For outputs:
>   * - %IGT_CONNECTOR_CRTC_ID
>   * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> + * - %IGT_CONNECTOR_WRITEBACK_FB_ID
> + * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
>   * - igt_output_override_mode() to default.
>   *
>   * For pipes:
> @@ -1970,6 +1982,8 @@ void igt_display_require(igt_display_t *display, int drm_fd)
>  
>  	display->drm_fd = drm_fd;
>  
> +	drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> +
>  	resources = drmModeGetResources(display->drm_fd);
>  	if (!resources)
>  		goto out;
> @@ -2263,6 +2277,11 @@ static void igt_output_fini(igt_output_t *output)
>  	kmstest_free_connector_config(&output->config);
>  	free(output->name);
>  	output->name = NULL;
> +
> +	if (output->writeback_out_fence_fd != -1) {
> +		close(output->writeback_out_fence_fd);
> +		output->writeback_out_fence_fd = -1;
> +	}
>  }
>  
>  /**
> @@ -3325,6 +3344,11 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
>  					  output->props[i],
>  					  output->values[i]));
>  	}
> +
> +	if (output->writeback_out_fence_fd != -1) {
> +		close(output->writeback_out_fence_fd);
> +		output->writeback_out_fence_fd = -1;
> +	}
>  }
>  
>  /*
> @@ -3447,6 +3471,16 @@ display_commit_changed(igt_display_t *display, enum igt_commit_style s)
>  		else
>  			/* no modeset in universal commit, no change to crtc. */
>  			output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
> +
> +		if (s == COMMIT_ATOMIC) {
> +			if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
> +				igt_assert(output->writeback_out_fence_fd >= 0);
> +
> +			output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
> +			output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
> +			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
> +			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
> +		}
>  	}
>  
>  	if (display->first_commit) {
> @@ -4119,6 +4153,29 @@ void igt_pipe_request_out_fence(igt_pipe_t *pipe)
>  	igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
>  }
>  
> +/**
> + * igt_output_set_writeback_fb:
> + * @output: Target output
> + * @fb: Target framebuffer
> + *
> + * This function sets the given @fb to be used as the target framebuffer for the
> + * writeback engine at the next atomic commit. It will also request a writeback
> + * out fence that will contain the fd number of the out fence created by KMS if
> + * the given @fb is valid.
> + */
> +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
> +{
> +	igt_display_t *display = output->display;
> +
> +	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
> +
> +	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
> +	/* only request a writeback out fence if the framebuffer is valid */
> +	if (fb)
> +		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> +					  (ptrdiff_t)&output->writeback_out_fence_fd);
> +}
> +
>  /**
>   * igt_wait_for_vblank_count:
>   * @drm_fd: A drm file descriptor
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index a448a003..cacc6b90 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -123,6 +123,9 @@ enum igt_atomic_connector_properties {
>         IGT_CONNECTOR_BROADCAST_RGB,
>         IGT_CONNECTOR_CONTENT_PROTECTION,
>         IGT_CONNECTOR_VRR_CAPABLE,
> +       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
> +       IGT_CONNECTOR_WRITEBACK_FB_ID,
> +       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
>         IGT_NUM_CONNECTOR_PROPS
>  };
>  
> @@ -364,6 +367,8 @@ typedef struct {
>  	bool use_override_mode;
>  	drmModeModeInfo override_mode;
>  
> +	int32_t writeback_out_fence_fd;
> +
>  	/* bitmask of changed properties */
>  	uint64_t changed;
>  
> @@ -414,6 +419,7 @@ igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
>  igt_output_t *igt_output_from_connector(igt_display_t *display,
>      drmModeConnector *connector);
>  const drmModeModeInfo *igt_std_1024_mode_get(void);
> +void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
>  
>  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
>  int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
> -- 
> 2.21.0
> 
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
diff mbox series

Patch

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index da188a39..140db346 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -325,6 +325,9 @@  const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
 	[IGT_CONNECTOR_BROADCAST_RGB] = "Broadcast RGB",
 	[IGT_CONNECTOR_CONTENT_PROTECTION] = "Content Protection",
 	[IGT_CONNECTOR_VRR_CAPABLE] = "vrr_capable",
+	[IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
+	[IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
+	[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
 };
 
 /*
@@ -557,6 +560,7 @@  static const struct type_name connector_type_names[] = {
 	{ DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" },
 	{ DRM_MODE_CONNECTOR_DSI, "DSI" },
 	{ DRM_MODE_CONNECTOR_DPI, "DPI" },
+	{ DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" },
 	{}
 };
 
@@ -1889,6 +1893,12 @@  static void igt_output_reset(igt_output_t *output)
 	if (igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
 		igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB,
 					  BROADCAST_RGB_FULL);
+	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
+		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
+	if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
+		igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
+		output->writeback_out_fence_fd = -1;
+	}
 }
 
 /**
@@ -1901,6 +1911,8 @@  static void igt_output_reset(igt_output_t *output)
  * For outputs:
  * - %IGT_CONNECTOR_CRTC_ID
  * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
+ * - %IGT_CONNECTOR_WRITEBACK_FB_ID
+ * - %IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR
  * - igt_output_override_mode() to default.
  *
  * For pipes:
@@ -1970,6 +1982,8 @@  void igt_display_require(igt_display_t *display, int drm_fd)
 
 	display->drm_fd = drm_fd;
 
+	drmSetClientCap(drm_fd, DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
+
 	resources = drmModeGetResources(display->drm_fd);
 	if (!resources)
 		goto out;
@@ -2263,6 +2277,11 @@  static void igt_output_fini(igt_output_t *output)
 	kmstest_free_connector_config(&output->config);
 	free(output->name);
 	output->name = NULL;
+
+	if (output->writeback_out_fence_fd != -1) {
+		close(output->writeback_out_fence_fd);
+		output->writeback_out_fence_fd = -1;
+	}
 }
 
 /**
@@ -3325,6 +3344,11 @@  static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
 					  output->props[i],
 					  output->values[i]));
 	}
+
+	if (output->writeback_out_fence_fd != -1) {
+		close(output->writeback_out_fence_fd);
+		output->writeback_out_fence_fd = -1;
+	}
 }
 
 /*
@@ -3447,6 +3471,16 @@  display_commit_changed(igt_display_t *display, enum igt_commit_style s)
 		else
 			/* no modeset in universal commit, no change to crtc. */
 			output->changed &= 1 << IGT_CONNECTOR_CRTC_ID;
+
+		if (s == COMMIT_ATOMIC) {
+			if (igt_output_is_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR))
+				igt_assert(output->writeback_out_fence_fd >= 0);
+
+			output->values[IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = 0;
+			output->values[IGT_CONNECTOR_WRITEBACK_FB_ID] = 0;
+			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_FB_ID);
+			igt_output_clear_prop_changed(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR);
+		}
 	}
 
 	if (display->first_commit) {
@@ -4119,6 +4153,29 @@  void igt_pipe_request_out_fence(igt_pipe_t *pipe)
 	igt_pipe_obj_set_prop_value(pipe, IGT_CRTC_OUT_FENCE_PTR, (ptrdiff_t)&pipe->out_fence_fd);
 }
 
+/**
+ * igt_output_set_writeback_fb:
+ * @output: Target output
+ * @fb: Target framebuffer
+ *
+ * This function sets the given @fb to be used as the target framebuffer for the
+ * writeback engine at the next atomic commit. It will also request a writeback
+ * out fence that will contain the fd number of the out fence created by KMS if
+ * the given @fb is valid.
+ */
+void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb)
+{
+	igt_display_t *display = output->display;
+
+	LOG(display, "%s: output_set_writeback_fb(%d)\n", output->name, fb ? fb->fb_id : 0);
+
+	igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, fb ? fb->fb_id : 0);
+	/* only request a writeback out fence if the framebuffer is valid */
+	if (fb)
+		igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
+					  (ptrdiff_t)&output->writeback_out_fence_fd);
+}
+
 /**
  * igt_wait_for_vblank_count:
  * @drm_fd: A drm file descriptor
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index a448a003..cacc6b90 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -123,6 +123,9 @@  enum igt_atomic_connector_properties {
        IGT_CONNECTOR_BROADCAST_RGB,
        IGT_CONNECTOR_CONTENT_PROTECTION,
        IGT_CONNECTOR_VRR_CAPABLE,
+       IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
+       IGT_CONNECTOR_WRITEBACK_FB_ID,
+       IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
        IGT_NUM_CONNECTOR_PROPS
 };
 
@@ -364,6 +367,8 @@  typedef struct {
 	bool use_override_mode;
 	drmModeModeInfo override_mode;
 
+	int32_t writeback_out_fence_fd;
+
 	/* bitmask of changed properties */
 	uint64_t changed;
 
@@ -414,6 +419,7 @@  igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
 igt_output_t *igt_output_from_connector(igt_display_t *display,
     drmModeConnector *connector);
 const drmModeModeInfo *igt_std_1024_mode_get(void);
+void igt_output_set_writeback_fb(igt_output_t *output, struct igt_fb *fb);
 
 igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
 int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);