diff mbox

[v12,08/10] drm: Expose modes with aspect ratio, only if requested

Message ID 1524831296-11198-9-git-send-email-ankit.k.nautiyal@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nautiyal, Ankit K April 27, 2018, 12:14 p.m. UTC
From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

We parse the EDID and add all the modes in the connector's modelist.
This adds CEA modes with aspect ratio information too, regadless of
whether user space requested this information or not.

This patch prunes the modes with aspect-ratio information, from a
connector's modelist, if the user-space has not set the aspect ratio
DRM client cap. However if such a mode is unique in the list, it is
kept in the list, with aspect-ratio flags reset.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Jose Abreu <jose.abreu@synopsys.com>

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

V3: As suggested by Ville, modified the mechanism of pruning of modes
    with aspect-ratio, if the aspect-ratio is not supported. Instead
    of straight away pruning such a mode, the mode is retained with
    aspect ratio bits set to zero, provided it is unique.
V4: rebase
V5: Addressed review comments from Ville:
    -used a pointer to store last valid mode.
    -avoided, modifying of picture_aspect_ratio in kernel mode,
     instead only flags bits of user mode are reset (if aspect-ratio
     is not supported).
V6: As suggested by Ville, corrected the mode pruning logic and
    elaborated the mode pruning logic and the assumptions taken.
V7: rebase
V8: rebase
V9: rebase
V10: rebase
V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
     logic to correctly identify and prune modes with aspect-ratio,
     if aspect-ratio cap is not set.
V12: As suggested by Ville, added another list_head in
     drm_mode_display to traverse the list of exposed modes and
     avoided duplication of modes.
---
 drivers/gpu/drm/drm_connector.c | 52 ++++++++++++++++++++++++++++++++++-------
 include/drm/drm_modes.h         |  9 +++++++
 2 files changed, 53 insertions(+), 8 deletions(-)

Comments

Ville Syrjälä April 27, 2018, 2:05 p.m. UTC | #1
On Fri, Apr 27, 2018 at 05:44:54PM +0530, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> We parse the EDID and add all the modes in the connector's modelist.
> This adds CEA modes with aspect ratio information too, regadless of
> whether user space requested this information or not.
> 
> This patch prunes the modes with aspect-ratio information, from a
> connector's modelist, if the user-space has not set the aspect ratio
> DRM client cap. However if such a mode is unique in the list, it is
> kept in the list, with aspect-ratio flags reset.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Jose Abreu <jose.abreu@synopsys.com>
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> 
> V3: As suggested by Ville, modified the mechanism of pruning of modes
>     with aspect-ratio, if the aspect-ratio is not supported. Instead
>     of straight away pruning such a mode, the mode is retained with
>     aspect ratio bits set to zero, provided it is unique.
> V4: rebase
> V5: Addressed review comments from Ville:
>     -used a pointer to store last valid mode.
>     -avoided, modifying of picture_aspect_ratio in kernel mode,
>      instead only flags bits of user mode are reset (if aspect-ratio
>      is not supported).
> V6: As suggested by Ville, corrected the mode pruning logic and
>     elaborated the mode pruning logic and the assumptions taken.
> V7: rebase
> V8: rebase
> V9: rebase
> V10: rebase
> V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
>      logic to correctly identify and prune modes with aspect-ratio,
>      if aspect-ratio cap is not set.
> V12: As suggested by Ville, added another list_head in
>      drm_mode_display to traverse the list of exposed modes and
>      avoided duplication of modes.
> ---
>  drivers/gpu/drm/drm_connector.c | 52 ++++++++++++++++++++++++++++++++++-------
>  include/drm/drm_modes.h         |  9 +++++++
>  2 files changed, 53 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b3cde89..e12964b 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1531,15 +1531,35 @@ static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne
>  	return connector->encoder;
>  }
>  
> -static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
> -					 const struct drm_file *file_priv)
> +static bool
> +drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
> +			     const struct list_head *modelist,
> +			     const struct drm_file *file_priv)
>  {
>  	/*
>  	 * If user-space hasn't configured the driver to expose the stereo 3D
>  	 * modes, don't expose them.
>  	 */
> +	struct drm_display_mode *mode_itr;
> +

This is clearly misplaced. It should be before the comment. Hmm, 
atually better move it into the aspect ratio if block since it's
not needed elsewhere.

>  	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
>  		return false;
> +	/*
> +	 * If user-space hasn't configured the driver to expose the modes
> +	 * with aspect-ratio, don't expose them. However if such a mode
> +	 * is unique, let it be exposed, but reset the aspect-ratio flags
> +	 * while preparing the list of user-modes.
> +	 */
> +	if (!file_priv->aspect_ratio_allowed &&
> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
> +		list_for_each_entry(mode_itr, modelist, exp_head)
> +			if (drm_mode_match(mode_itr, mode,
> +					   DRM_MODE_MATCH_TIMINGS |
> +					   DRM_MODE_MATCH_CLOCK |
> +					   DRM_MODE_MATCH_FLAGS |
> +					   DRM_MODE_MATCH_3D_FLAGS))
> +				return false;
> +	}
>  
>  	return true;
>  }
> @@ -1550,7 +1570,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	struct drm_mode_get_connector *out_resp = data;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> -	struct drm_display_mode *mode;
> +	struct drm_display_mode *mode, *tmp;
> +	struct list_head modelist;

LIST_HEAD_INIT(modelist);

And maybe call it export_list or something like that.

>  	int mode_count = 0;
>  	int encoders_count = 0;
>  	int ret = 0;
> @@ -1605,23 +1626,34 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	out_resp->subpixel = connector->display_info.subpixel_order;
>  	out_resp->connection = connector->status;
>  
> +	INIT_LIST_HEAD(&modelist);
> +
>  	/* delayed so we get modes regardless of pre-fill_modes state */
>  	list_for_each_entry(mode, &connector->modes, head)
> -		if (drm_mode_expose_to_userspace(mode, file_priv))
> +		if (drm_mode_expose_to_userspace(mode, &modelist,
> +						 file_priv)) {
> +			list_add_tail(&mode->exp_head, &modelist);
>  			mode_count++;
> +		}
>  
>  	/*
>  	 * This ioctl is called twice, once to determine how much space is
>  	 * needed, and the 2nd time to fill it.
> +	 * The modes that need to be exposed to the user are maintained in the
> +	 * 'modelist'. When the ioctl is called first time to determine the,
> +	 * space, the modelist gets filled, to find the no. of modes. In the
> +	 * 2nd time, the user modes are filled, one by one from the modelist.
>  	 */
>  	if ((out_resp->count_modes >= mode_count) && mode_count) {
>  		copied = 0;
>  		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
> -		list_for_each_entry(mode, &connector->modes, head) {
> -			if (!drm_mode_expose_to_userspace(mode, file_priv))
> -				continue;
> -
> +		list_for_each_entry(mode, &modelist, exp_head) {
>  			drm_mode_convert_to_umode(&u_mode, mode);
> +			/*
> +			 * Reset aspect ratio flags of user-mode, if modes with
> +			 * aspect-ratio are not supported.
> +			 */
> +			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
>  			if (copy_to_user(mode_ptr + copied,
>  					 &u_mode, sizeof(u_mode))) {
>  				ret = -EFAULT;
> @@ -1651,6 +1683,10 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>  
>  out:
> +	list_for_each_entry_safe(mode, tmp, &modelist, exp_head)
> +		list_del(&mode->exp_head);
> +	list_del(&modelist);

That last list_del() doesn't make sense. Also you can't touch the
mode->exp_head without locking. We should document that it's
protected by mode_config.mutex.

Since we don't actually care about exp_head outside of this function
I think we can just leave the stale pointers in place. No one else is
supposed to look at them, a fact we should probaly also include in the
documentation.

> +
>  	drm_connector_put(connector);
>  
>  	return ret;
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index e0b060d..1888ec7 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -411,6 +411,15 @@ struct drm_display_mode {
>  	 * Field for setting the HDMI picture aspect ratio of a mode.
>  	 */
>  	enum hdmi_picture_aspect picture_aspect_ratio;
> +
> +	/**
> +	 * @exp_head:

I think we can afford to write 'export_head'.

> +	 *
> +	 * struct list_head for modes to be exposed to the userspace.
> +	 *
> +	 */
> +	struct list_head exp_head;
> +
>  };
>  
>  /**
> -- 
> 2.7.4
Nautiyal, Ankit K April 30, 2018, 6:52 a.m. UTC | #2
On 4/27/2018 7:35 PM, Ville Syrjälä wrote:
> On Fri, Apr 27, 2018 at 05:44:54PM +0530, Nautiyal, Ankit K wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> We parse the EDID and add all the modes in the connector's modelist.
>> This adds CEA modes with aspect ratio information too, regadless of
>> whether user space requested this information or not.
>>
>> This patch prunes the modes with aspect-ratio information, from a
>> connector's modelist, if the user-space has not set the aspect ratio
>> DRM client cap. However if such a mode is unique in the list, it is
>> kept in the list, with aspect-ratio flags reset.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> V3: As suggested by Ville, modified the mechanism of pruning of modes
>>      with aspect-ratio, if the aspect-ratio is not supported. Instead
>>      of straight away pruning such a mode, the mode is retained with
>>      aspect ratio bits set to zero, provided it is unique.
>> V4: rebase
>> V5: Addressed review comments from Ville:
>>      -used a pointer to store last valid mode.
>>      -avoided, modifying of picture_aspect_ratio in kernel mode,
>>       instead only flags bits of user mode are reset (if aspect-ratio
>>       is not supported).
>> V6: As suggested by Ville, corrected the mode pruning logic and
>>      elaborated the mode pruning logic and the assumptions taken.
>> V7: rebase
>> V8: rebase
>> V9: rebase
>> V10: rebase
>> V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
>>       logic to correctly identify and prune modes with aspect-ratio,
>>       if aspect-ratio cap is not set.
>> V12: As suggested by Ville, added another list_head in
>>       drm_mode_display to traverse the list of exposed modes and
>>       avoided duplication of modes.
>> ---
>>   drivers/gpu/drm/drm_connector.c | 52 ++++++++++++++++++++++++++++++++++-------
>>   include/drm/drm_modes.h         |  9 +++++++
>>   2 files changed, 53 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index b3cde89..e12964b 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1531,15 +1531,35 @@ static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne
>>   	return connector->encoder;
>>   }
>>   
>> -static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
>> -					 const struct drm_file *file_priv)
>> +static bool
>> +drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
>> +			     const struct list_head *modelist,
>> +			     const struct drm_file *file_priv)
>>   {
>>   	/*
>>   	 * If user-space hasn't configured the driver to expose the stereo 3D
>>   	 * modes, don't expose them.
>>   	 */
>> +	struct drm_display_mode *mode_itr;
>> +
> This is clearly misplaced. It should be before the comment. Hmm,
> atually better move it into the aspect ratio if block since it's
> not needed elsewhere.

That makes sense, will move it to the aspect-ratio block.

>
>>   	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
>>   		return false;
>> +	/*
>> +	 * If user-space hasn't configured the driver to expose the modes
>> +	 * with aspect-ratio, don't expose them. However if such a mode
>> +	 * is unique, let it be exposed, but reset the aspect-ratio flags
>> +	 * while preparing the list of user-modes.
>> +	 */
>> +	if (!file_priv->aspect_ratio_allowed &&
>> +	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
>> +		list_for_each_entry(mode_itr, modelist, exp_head)
>> +			if (drm_mode_match(mode_itr, mode,
>> +					   DRM_MODE_MATCH_TIMINGS |
>> +					   DRM_MODE_MATCH_CLOCK |
>> +					   DRM_MODE_MATCH_FLAGS |
>> +					   DRM_MODE_MATCH_3D_FLAGS))
>> +				return false;
>> +	}
>>   
>>   	return true;
>>   }
>> @@ -1550,7 +1570,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>>   	struct drm_mode_get_connector *out_resp = data;
>>   	struct drm_connector *connector;
>>   	struct drm_encoder *encoder;
>> -	struct drm_display_mode *mode;
>> +	struct drm_display_mode *mode, *tmp;
>> +	struct list_head modelist;
> LIST_HEAD_INIT(modelist);
>
> And maybe call it export_list or something like that.

Alright. Will change the name to export_list.

I hope you mean LIST_HEAD(export_list);

>>   	int mode_count = 0;
>>   	int encoders_count = 0;
>>   	int ret = 0;
>> @@ -1605,23 +1626,34 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>>   	out_resp->subpixel = connector->display_info.subpixel_order;
>>   	out_resp->connection = connector->status;
>>   
>> +	INIT_LIST_HEAD(&modelist);
>> +
>>   	/* delayed so we get modes regardless of pre-fill_modes state */
>>   	list_for_each_entry(mode, &connector->modes, head)
>> -		if (drm_mode_expose_to_userspace(mode, file_priv))
>> +		if (drm_mode_expose_to_userspace(mode, &modelist,
>> +						 file_priv)) {
>> +			list_add_tail(&mode->exp_head, &modelist);
>>   			mode_count++;
>> +		}
>>   
>>   	/*
>>   	 * This ioctl is called twice, once to determine how much space is
>>   	 * needed, and the 2nd time to fill it.
>> +	 * The modes that need to be exposed to the user are maintained in the
>> +	 * 'modelist'. When the ioctl is called first time to determine the,
>> +	 * space, the modelist gets filled, to find the no. of modes. In the
>> +	 * 2nd time, the user modes are filled, one by one from the modelist.
>>   	 */
>>   	if ((out_resp->count_modes >= mode_count) && mode_count) {
>>   		copied = 0;
>>   		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
>> -		list_for_each_entry(mode, &connector->modes, head) {
>> -			if (!drm_mode_expose_to_userspace(mode, file_priv))
>> -				continue;
>> -
>> +		list_for_each_entry(mode, &modelist, exp_head) {
>>   			drm_mode_convert_to_umode(&u_mode, mode);
>> +			/*
>> +			 * Reset aspect ratio flags of user-mode, if modes with
>> +			 * aspect-ratio are not supported.
>> +			 */
>> +			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
>>   			if (copy_to_user(mode_ptr + copied,
>>   					 &u_mode, sizeof(u_mode))) {
>>   				ret = -EFAULT;
>> @@ -1651,6 +1683,10 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>>   	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>   
>>   out:
>> +	list_for_each_entry_safe(mode, tmp, &modelist, exp_head)
>> +		list_del(&mode->exp_head);
>> +	list_del(&modelist);
> That last list_del() doesn't make sense. Also you can't touch the
> mode->exp_head without locking. We should document that it's
> protected by mode_config.mutex.
Oh yes, list_del () is not required, the deletion of the last mode, will 
automatically
set the next and prev of the list_head to itself.
> Since we don't actually care about exp_head outside of this function
> I think we can just leave the stale pointers in place. No one else is
> supposed to look at them, a fact we should probaly also include in the
> documentation.

Ok, so I will let the stale pointers in place, and explain the reasoning 
as comments.
>> +
>>   	drm_connector_put(connector);
>>   
>>   	return ret;
>> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
>> index e0b060d..1888ec7 100644
>> --- a/include/drm/drm_modes.h
>> +++ b/include/drm/drm_modes.h
>> @@ -411,6 +411,15 @@ struct drm_display_mode {
>>   	 * Field for setting the HDMI picture aspect ratio of a mode.
>>   	 */
>>   	enum hdmi_picture_aspect picture_aspect_ratio;
>> +
>> +	/**
>> +	 * @exp_head:
> I think we can afford to write 'export_head'.

Yeah right, shortening export_head to exp_head didn't do much :)
Will change it to export_head.

Regards,
Ankit

>> +	 *
>> +	 * struct list_head for modes to be exposed to the userspace.
>> +	 *
>> +	 */
>> +	struct list_head exp_head;
>> +
>>   };
>>   
>>   /**
>> -- 
>> 2.7.4
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b3cde89..e12964b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1531,15 +1531,35 @@  static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne
 	return connector->encoder;
 }
 
-static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
-					 const struct drm_file *file_priv)
+static bool
+drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
+			     const struct list_head *modelist,
+			     const struct drm_file *file_priv)
 {
 	/*
 	 * If user-space hasn't configured the driver to expose the stereo 3D
 	 * modes, don't expose them.
 	 */
+	struct drm_display_mode *mode_itr;
+
 	if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
 		return false;
+	/*
+	 * If user-space hasn't configured the driver to expose the modes
+	 * with aspect-ratio, don't expose them. However if such a mode
+	 * is unique, let it be exposed, but reset the aspect-ratio flags
+	 * while preparing the list of user-modes.
+	 */
+	if (!file_priv->aspect_ratio_allowed &&
+	    mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
+		list_for_each_entry(mode_itr, modelist, exp_head)
+			if (drm_mode_match(mode_itr, mode,
+					   DRM_MODE_MATCH_TIMINGS |
+					   DRM_MODE_MATCH_CLOCK |
+					   DRM_MODE_MATCH_FLAGS |
+					   DRM_MODE_MATCH_3D_FLAGS))
+				return false;
+	}
 
 	return true;
 }
@@ -1550,7 +1570,8 @@  int drm_mode_getconnector(struct drm_device *dev, void *data,
 	struct drm_mode_get_connector *out_resp = data;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
-	struct drm_display_mode *mode;
+	struct drm_display_mode *mode, *tmp;
+	struct list_head modelist;
 	int mode_count = 0;
 	int encoders_count = 0;
 	int ret = 0;
@@ -1605,23 +1626,34 @@  int drm_mode_getconnector(struct drm_device *dev, void *data,
 	out_resp->subpixel = connector->display_info.subpixel_order;
 	out_resp->connection = connector->status;
 
+	INIT_LIST_HEAD(&modelist);
+
 	/* delayed so we get modes regardless of pre-fill_modes state */
 	list_for_each_entry(mode, &connector->modes, head)
-		if (drm_mode_expose_to_userspace(mode, file_priv))
+		if (drm_mode_expose_to_userspace(mode, &modelist,
+						 file_priv)) {
+			list_add_tail(&mode->exp_head, &modelist);
 			mode_count++;
+		}
 
 	/*
 	 * This ioctl is called twice, once to determine how much space is
 	 * needed, and the 2nd time to fill it.
+	 * The modes that need to be exposed to the user are maintained in the
+	 * 'modelist'. When the ioctl is called first time to determine the,
+	 * space, the modelist gets filled, to find the no. of modes. In the
+	 * 2nd time, the user modes are filled, one by one from the modelist.
 	 */
 	if ((out_resp->count_modes >= mode_count) && mode_count) {
 		copied = 0;
 		mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
-		list_for_each_entry(mode, &connector->modes, head) {
-			if (!drm_mode_expose_to_userspace(mode, file_priv))
-				continue;
-
+		list_for_each_entry(mode, &modelist, exp_head) {
 			drm_mode_convert_to_umode(&u_mode, mode);
+			/*
+			 * Reset aspect ratio flags of user-mode, if modes with
+			 * aspect-ratio are not supported.
+			 */
+			drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
 			if (copy_to_user(mode_ptr + copied,
 					 &u_mode, sizeof(u_mode))) {
 				ret = -EFAULT;
@@ -1651,6 +1683,10 @@  int drm_mode_getconnector(struct drm_device *dev, void *data,
 	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 
 out:
+	list_for_each_entry_safe(mode, tmp, &modelist, exp_head)
+		list_del(&mode->exp_head);
+	list_del(&modelist);
+
 	drm_connector_put(connector);
 
 	return ret;
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index e0b060d..1888ec7 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -411,6 +411,15 @@  struct drm_display_mode {
 	 * Field for setting the HDMI picture aspect ratio of a mode.
 	 */
 	enum hdmi_picture_aspect picture_aspect_ratio;
+
+	/**
+	 * @exp_head:
+	 *
+	 * struct list_head for modes to be exposed to the userspace.
+	 *
+	 */
+	struct list_head exp_head;
+
 };
 
 /**