[PATCH v4 3/5] drm/dp-mst: Add MST support to DP DPCD R/W functions

[Lyude Paul]

Mhhh, one last nitpick

On Thu, 2019-08-22 at 09:57 -0400, David Francis wrote:
> Instead of having drm_dp_dpcd_read/write and
> drm_dp_mst_dpcd_read/write as entry points into the
> aux code, have drm_dp_dpcd_read/write handle both.
> 
> This means that DRM drivers can make MST DPCD read/writes.
> 
> v2: Fix spacing
> 
> Cc: Leo Li <sunpeng.li at amd.com>
> Cc: Lyude Paul <lyude at redhat.com>
> Signed-off-by: David Francis <David.Francis at amd.com>
> ---
>  drivers/gpu/drm/drm_dp_aux_dev.c | 12 ++----------
>  drivers/gpu/drm/drm_dp_helper.c  | 10 ++++++++--
>  2 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> b/drivers/gpu/drm/drm_dp_aux_dev.c
> index 0cfb386754c3..418cad4f649a 100644
> --- a/drivers/gpu/drm/drm_dp_aux_dev.c
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -163,11 +163,7 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb,
> struct iov_iter *to)
>  			break;
>  		}
>  
> -		if (aux_dev->aux->is_remote)
> -			res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf,
> -						   todo);
> -		else
> -			res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
> +		res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
>  
>  		if (res <= 0)
>  			break;
> @@ -215,11 +211,7 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb,
> struct iov_iter *from)
>  			break;
>  		}
>  
> -		if (aux_dev->aux->is_remote)
> -			res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf,
> -						    todo);
> -		else
> -			res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
> +		res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf, todo);
>  
>  		if (res <= 0)
>  			break;
> diff --git a/drivers/gpu/drm/drm_dp_helper.c
> b/drivers/gpu/drm/drm_dp_helper.c
> index ffc68d305afe..b1599760b6a3 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -30,6 +30,7 @@
>  #include <linux/seq_file.h>
>  
>  #include <drm/drm_dp_helper.h>
> +#include <drm/drm_dp_mst_helper.h>
>  #include <drm/drm_print.h>
>  #include <drm/drm_vblank.h>
>  
> @@ -251,7 +252,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
> request,
>  
>  /**
>   * drm_dp_dpcd_read() - read a series of bytes from the DPCD
> - * @aux: DisplayPort AUX channel
> + * @aux: DisplayPort AUX channel (SST or MST)
>   * @offset: address of the (first) register to read
>   * @buffer: buffer to store the register values
>   * @size: number of bytes in @buffer
> @@ -268,6 +269,8 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux,
> unsigned int offset,
>  {
>  	int ret;
>  
> +	if (aux->is_remote)
> +		return drm_dp_mst_dpcd_read(aux, offset, buffer, size);

Let's actually replace this with

if (aux->is_remote)
	ret = drm_dp_mst_dpcd_read(aux, offset, buffer, size);
else
	ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer,
	                         size);

That way we still call drm_dp_dump_access() so that we can log the DPCD read
to the kernel log.

Let's also add a space down here ⬇...
>  	/*
>  	 * HP ZR24w corrupts the first DPCD access after entering power save
>  	 * mode. Eg. on a read, the entire buffer will be filled with the same
> @@ -296,7 +299,7 @@ EXPORT_SYMBOL(drm_dp_dpcd_read);
>  
>  /**
>   * drm_dp_dpcd_write() - write a series of bytes to the DPCD
> - * @aux: DisplayPort AUX channel
> + * @aux: DisplayPort AUX channel (SST or MST)
>   * @offset: address of the (first) register to write
>   * @buffer: buffer containing the values to write
>   * @size: number of bytes in @buffer
> @@ -313,6 +316,9 @@ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux,
> unsigned int offset,
>  {
>  	int ret;
>  
> +	if (aux->is_remote)
> +		return drm_dp_mst_dpcd_write(aux, offset, buffer, size);
> +

...and do the same thing with drm_dp_dpcd_write() that we did with
drm_dp_dpcd_read()

With those changes, this patch is:

Reviewed-by: Lyude Paul <lyude at redhat.com>
>  	ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer,
>  				 size);
>  	drm_dp_dump_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer, ret);
-- 
Cheers,
	Lyude Paul