Re: [igt-dev] [PATCH V6 i-g-t 1/6] lib/igt_kms: Add writeback support

["Ser, Simon" <simon.ser@intel.com>]

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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev