Re: [PATCH 2/6] drm: add tile_group support. (v2)

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Oct 22, 2014 at 12:32:03PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> A tile group is an identifier shared by a single monitor,
> DisplayID topology has 8 bytes we can use for this, just
> use those for now until something else comes up in the
> future. We assign these to an idr and use the idr to
> tell userspace what connectors are in the same tile group.
> 
> DisplayID v1.3 says the serial number must be unique for
> displays from the same manufacturer.
> 
> v2:
> destroy idr (dvdhrm)
> add docbook (danvet)
> airlied:- not sure how to make docbook add fns to tile group section.

Either you have to extract them into a new file or you have to list them
all explicitly. The kerneldoc nano howto has the various options you can
use. Thus far we haven't documented drm-internal functions though, only
those exported to drivers or helpers as guidelines to driver writers. Not
stopping you ofc ;-) But imo just documenting the tile prop registration
function is good enough.

wrt the patch I'm not 100% sure the kref_get_unless_zero is perfectly
race-free, but that depends upon how we solve the hotplugging of
properties and stuff I think.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  Documentation/DocBook/drm.tmpl |  4 ++
>  drivers/gpu/drm/drm_crtc.c     | 99 ++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h         | 16 +++++++
>  3 files changed, 119 insertions(+)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 0a5cbbb..5ea6289 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2374,6 +2374,10 @@ void intel_crt_init(struct drm_device *dev)
>        <title id="drm-kms-planehelpers">Plane Helper Reference</title>
>  !Edrivers/gpu/drm/drm_plane_helper.c Plane Helpers
>      </sect2>
> +    <sect2>
> +	  <title>Tile group</title>
> +!Pdrivers/gpu/drm/drm_crtc.c Tile group
> +    </sect2>
>    </sect1>
>  
>    <!-- Internals: kms properties -->
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 363301c..7f45fdc 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5047,6 +5047,7 @@ void drm_mode_config_init(struct drm_device *dev)
>  	INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
>  	INIT_LIST_HEAD(&dev->mode_config.plane_list);
>  	idr_init(&dev->mode_config.crtc_idr);
> +	idr_init(&dev->mode_config.tile_idr);
>  
>  	drm_modeset_lock_all(dev);
>  	drm_mode_create_standard_connector_properties(dev);
> @@ -5134,6 +5135,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>  		crtc->funcs->destroy(crtc);
>  	}
>  
> +	idr_destroy(&dev->mode_config.tile_idr);
>  	idr_destroy(&dev->mode_config.crtc_idr);
>  	drm_modeset_lock_fini(&dev->mode_config.connection_mutex);
>  }
> @@ -5156,3 +5158,100 @@ struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
>  					   supported_rotations);
>  }
>  EXPORT_SYMBOL(drm_mode_create_rotation_property);
> +
> +/**
> + * DOC: Tile group
> + *
> + * Tile groups are used to represent tiled monitors with a unique
> + * integer identifier. Tiled monitors using DisplayID v1.3 have
> + * a unique 8-byte handle, we store this in a tile group, so we
> + * have a common identifier for all tiles in a monitor group.
> + */
> +static void drm_tile_group_free(struct kref *kref)
> +{
> +	struct drm_tile_group *tg = container_of(kref, struct drm_tile_group, refcount);
> +	struct drm_device *dev = tg->dev;
> +	mutex_lock(&dev->mode_config.idr_mutex);
> +	idr_remove(&dev->mode_config.tile_idr, tg->id);
> +	mutex_lock(&dev->mode_config.idr_mutex);
> +	kfree(tg);
> +}
> +
> +/**
> + * drm_mode_put_tile_group - drop a reference to a tile group.
> + * @dev: DRM device
> + * @tg: tile group to drop reference to.
> + *
> + * drop reference to tile group and free if 0.
> + */
> +void drm_mode_put_tile_group(struct drm_device *dev,
> +			     struct drm_tile_group *tg)
> +{
> +	kref_put(&tg->refcount, drm_tile_group_free);
> +}
> +
> +/**
> + * drm_mode_get_tile_group - get a reference to an existing tile group
> + * @dev: DRM device
> + * @topology: 8-bytes unique per monitor.
> + *
> + * Use the unique bytes to get a reference to an existing tile group.
> + *
> + * RETURNS:
> + * tile group or NULL if not found.
> + */
> +struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
> +					       char topology[8])
> +{
> +	struct drm_tile_group *tg;
> +	int id;
> +	mutex_lock(&dev->mode_config.idr_mutex);
> +	idr_for_each_entry(&dev->mode_config.tile_idr, tg, id) {
> +		if (!memcmp(tg->group_data, topology, 8)) {
> +			if (!kref_get_unless_zero(&tg->refcount))
> +				tg = NULL;
> +			mutex_unlock(&dev->mode_config.idr_mutex);
> +			return tg;
> +		}
> +	}
> +	mutex_unlock(&dev->mode_config.idr_mutex);
> +	return NULL;
> +}
> +
> +/**
> + * drm_mode_create_tile_group - create a tile group from a displayid description
> + * @dev: DRM device
> + * @topology: 8-bytes unique per monitor.
> + *
> + * Create a tile group for the unique monitor, and get a unique
> + * identifier for the tile group.
> + *
> + * RETURNS:
> + * new tile group or error.
> + */
> +struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
> +						  char topology[8])
> +{
> +	struct drm_tile_group *tg;
> +	int ret;
> +
> +	tg = kzalloc(sizeof(*tg), GFP_KERNEL);
> +	if (!tg)
> +		return ERR_PTR(-ENOMEM);
> +
> +	kref_init(&tg->refcount);
> +	memcpy(tg->group_data, topology, 8);
> +	tg->dev = dev;
> +
> +	mutex_lock(&dev->mode_config.idr_mutex);
> +	ret = idr_alloc(&dev->mode_config.tile_idr, tg, 1, 0, GFP_KERNEL);
> +	if (ret >= 0) {
> +		tg->id = ret;
> +	} else {
> +		kfree(tg);
> +		tg = ERR_PTR(ret);
> +	}
> +
> +	mutex_unlock(&dev->mode_config.idr_mutex);
> +	return tg;
> +}
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index f1105d0..afaec4b 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -136,6 +136,14 @@ struct drm_display_info {
>  	u8 cea_rev;
>  };
>  
> +/* data corresponds to displayid vend/prod/serial */
> +struct drm_tile_group {
> +	struct kref refcount;
> +	struct drm_device *dev;
> +	int id;
> +	u8 group_data[8];
> +};
> +
>  struct drm_framebuffer_funcs {
>  	/* note: use drm_framebuffer_remove() */
>  	void (*destroy)(struct drm_framebuffer *framebuffer);
> @@ -770,6 +778,7 @@ struct drm_mode_config {
>  	struct drm_modeset_acquire_ctx *acquire_ctx; /* for legacy _lock_all() / _unlock_all() */
>  	struct mutex idr_mutex; /* for IDR management */
>  	struct idr crtc_idr; /* use this idr for all IDs, fb, crtc, connector, modes - just makes life easier */
> +	struct idr tile_idr; /* use this idr for all IDs, fb, crtc, connector, modes - just makes life easier */
>  	/* this is limited to one for now */
>  
>  
> @@ -1106,6 +1115,13 @@ extern void drm_set_preferred_mode(struct drm_connector *connector,
>  extern int drm_edid_header_is_valid(const u8 *raw_edid);
>  extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
>  extern bool drm_edid_is_valid(struct edid *edid);
> +
> +extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
> +							 char topology[8]);
> +extern struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
> +					       char topology[8]);
> +extern void drm_mode_put_tile_group(struct drm_device *dev,
> +				   struct drm_tile_group *tg);
>  struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
>  					   int hsize, int vsize, int fresh,
>  					   bool rb);
> -- 
> 2.1.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch