On Wed, Oct 26, 2016 at 09:55:07AM +0100, Brian Starkey wrote:
> Add the OUT_FENCE_PTR property to writeback connectors, to enable
> userspace to get a fence which will signal once the writeback is
> complete.
>
> A timeline is added to drm_connector for use by the writeback
> out-fences. It is up to drivers to check for a fence in the
> connector_state and signal the it appropriately when their writeback has
> finished.
>
> It is not allowed to request an out-fence without a framebuffer attached
> to the connector.
>
> Signed-off-by: Brian Starkey <brian.starkey@xxxxxxx>
Ah, here it is, so much for reading patches strictly in-order ;-) One
small comment below, otherwise I think this looks good. Again review from
Gustavo for the out fences stuff would be really good (so pls cc him). And
I think some igt testcases to exercise all the corner-cases in here.
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index a5e3778..7d40537 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -199,6 +199,7 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info,
> * @best_encoder: can be used by helpers and drivers to select the encoder
> * @state: backpointer to global drm_atomic_state
> * @fb: Writeback framebuffer, for DRM_MODE_CONNECTOR_WRITEBACK
> + * @out_fence: Fence which will clear when the framebuffer write has finished
> */
> struct drm_connector_state {
> struct drm_connector *connector;
> @@ -216,6 +217,9 @@ struct drm_connector_state {
> struct drm_atomic_state *state;
>
> struct drm_framebuffer *fb; /* do not write directly, use drm_atomic_set_fb_for_connector() */
btw if you feel like adding a 2nd comment in-line like above, then that's
a clear signal that you should move your kerneldoc struct member comments
to the inline style. You can freely mix&match inline with top-level struct
member documentation, so no need to change them all. You also missed the
doc for out_fence_ptr, 0day won't like that.
> +
> + struct fence *out_fence;
> + u64 __user *out_fence_ptr;
writeback_ prefix for both imo, like in patch 1.
> };
>
> /**
> @@ -546,6 +550,10 @@ struct drm_cmdline_mode {
> * @tile_v_loc: vertical location of this tile
> * @tile_h_size: horizontal size of this tile.
> * @tile_v_size: vertical size of this tile.
> + * @fence_context: context for fence signalling
> + * @fence_lock: fence lock for the fence context
> + * @fence_seqno: seqno variable to create fences
> + * @timeline_name: fence timeline name
> *
> * Each connector may be connected to one or more CRTCs, or may be clonable by
> * another connector if they can share a CRTC. Each connector also has a specific
> @@ -694,6 +702,12 @@ struct drm_connector {
> uint8_t num_h_tile, num_v_tile;
> uint8_t tile_h_loc, tile_v_loc;
> uint16_t tile_h_size, tile_v_size;
> +
> + /* fence timelines info for DRM out-fences */
> + unsigned int fence_context;
> + spinlock_t fence_lock;
> + unsigned long fence_seqno;
> + char timeline_name[32];
Should all have writeout_ prefix. And at that point I wonder a bit whether
we shouldn't just go ahead and create a struct drm_writeout_connector, to
keep this stuff nicely separate. Only change visible to drivers would be
the type of drm_writeback_connector_init, and they'd need to
allocate/embedd a different struct. Worth it imo.
-Daniel
> };
>
> #define obj_to_connector(x) container_of(x, struct drm_connector, base)
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index afdc2742..01f33bc 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -16,4 +16,6 @@ int drm_writeback_connector_init(struct drm_device *dev,
> const struct drm_connector_funcs *funcs,
> u32 *formats, int n_formats);
>
> +struct fence *drm_writeback_get_out_fence(struct drm_connector *connector,
> + struct drm_connector_state *conn_state);
> #endif
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
