diff mbox

[3/8] drm/i915: Use EAGAIN instead EBUSY for aux retry.

Message ID 1448066790-19591-4-git-send-email-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi Nov. 21, 2015, 12:46 a.m. UTC
Current EBUSY meaning is immediately retry, but this is
about to change. DRM aux transfer is about to change and
make EAGAIN the immediately retry and use EBUSY to wait
a bit for aux channels to recover for any error or wake up.

This has no functional change if the EAGAIN support is in
place already for drm aux transfer.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Nov. 23, 2015, 9:02 a.m. UTC | #1
On Fri, Nov 20, 2015 at 04:46:25PM -0800, Rodrigo Vivi wrote:
> Current EBUSY meaning is immediately retry, but this is
> about to change. DRM aux transfer is about to change and
> make EAGAIN the immediately retry and use EBUSY to wait
> a bit for aux channels to recover for any error or wake up.
> 
> This has no functional change if the EAGAIN support is in
> place already for drm aux transfer.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Please document the EAGAIN and EBUSY return codes for the ->transfer
member fo drm_dp_aux in the kerneldoc properly. While at it would be great
if you could document any other error codes that are treated specially for
this function.

Since this will be a bit more text please convert the kerneldoc to the new
per-member comment layout that 4.4 supports, i.e.

/**
 * struct foo - foo structure
 *
 * top-level blabla
 */
struct foo {
	/**
	 * @bar:
	 *
	 * Long text (with paragraphs) explaining bar.
	 */
};

Otherwise this looks really tidy and I really like how this allows us to
get rid of the intel_read_wake hack.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index bec443a..ed07f0a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -835,7 +835,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  			last_status = status;
>  		}
>  
> -		ret = -EBUSY;
> +		ret = -EAGAIN;
>  		goto out;
>  	}
>  
> @@ -890,7 +890,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  
>  	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
>  		DRM_ERROR("dp_aux_ch not done status 0x%08x\n", status);
> -		ret = -EBUSY;
> +		ret = -EAGAIN;
>  		goto out;
>  	}
>  
> -- 
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Jani Nikula Nov. 23, 2015, 9:41 a.m. UTC | #2
On Mon, 23 Nov 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Nov 20, 2015 at 04:46:25PM -0800, Rodrigo Vivi wrote:
>> Current EBUSY meaning is immediately retry, but this is
>> about to change. DRM aux transfer is about to change and
>> make EAGAIN the immediately retry and use EBUSY to wait
>> a bit for aux channels to recover for any error or wake up.
>> 
>> This has no functional change if the EAGAIN support is in
>> place already for drm aux transfer.
>> 
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Please document the EAGAIN and EBUSY return codes for the ->transfer
> member fo drm_dp_aux in the kerneldoc properly. While at it would be great
> if you could document any other error codes that are treated specially for
> this function.

See my comment in reply to patch 1 about the difference in handling of
the ->transfer return values for native aux and i2c-over-aux.

BR,
Jani.

>
> Since this will be a bit more text please convert the kerneldoc to the new
> per-member comment layout that 4.4 supports, i.e.
>
> /**
>  * struct foo - foo structure
>  *
>  * top-level blabla
>  */
> struct foo {
> 	/**
> 	 * @bar:
> 	 *
> 	 * Long text (with paragraphs) explaining bar.
> 	 */
> };
>
> Otherwise this looks really tidy and I really like how this allows us to
> get rid of the intel_read_wake hack.
> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index bec443a..ed07f0a 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -835,7 +835,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>>  			last_status = status;
>>  		}
>>  
>> -		ret = -EBUSY;
>> +		ret = -EAGAIN;
>>  		goto out;
>>  	}
>>  
>> @@ -890,7 +890,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>>  
>>  	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
>>  		DRM_ERROR("dp_aux_ch not done status 0x%08x\n", status);
>> -		ret = -EBUSY;
>> +		ret = -EAGAIN;
>>  		goto out;
>>  	}
>>  
>> -- 
>> 2.4.3
>> 
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bec443a..ed07f0a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -835,7 +835,7 @@  intel_dp_aux_ch(struct intel_dp *intel_dp,
 			last_status = status;
 		}
 
-		ret = -EBUSY;
+		ret = -EAGAIN;
 		goto out;
 	}
 
@@ -890,7 +890,7 @@  intel_dp_aux_ch(struct intel_dp *intel_dp,
 
 	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
 		DRM_ERROR("dp_aux_ch not done status 0x%08x\n", status);
-		ret = -EBUSY;
+		ret = -EAGAIN;
 		goto out;
 	}