diff mbox

[v2,7/8] drm/i915: Keep the AKSV details in intel_dp_hdcp_write_an_aksv()

Message ID 20180222212732.4665-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Feb. 22, 2018, 9:27 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's try to keep the details on the AKSV stuff concentrated
in one place. So move the control bit and +5 data size handling
there.

v2: Increase txbuf[] to include the payload which intel_dp_aux_xfer()
    will still load into the registers even though the hardware
    will ignore it

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 42 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 29 deletions(-)

Comments

Ramalingam C Feb. 23, 2018, 11:10 a.m. UTC | #1
This is really making it cleaner.

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>



On Friday 23 February 2018 02:57 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's try to keep the details on the AKSV stuff concentrated
> in one place. So move the control bit and +5 data size handling
> there.
>
> v2: Increase txbuf[] to include the payload which intel_dp_aux_xfer()
>      will still load into the registers even though the hardware
>      will ignore it
>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 42 +++++++++++++----------------------------
>   1 file changed, 13 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 217cc6aee477..0d699d230b77 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1059,29 +1059,11 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>   	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
>   }
>   
> -static uint32_t intel_dp_get_aux_send_ctl(struct intel_dp *intel_dp,
> -					  bool has_aux_irq,
> -					  int send_bytes,
> -					  uint32_t aux_clock_divider,
> -					  bool aksv_write)
> -{
> -	uint32_t val = 0;
> -
> -	if (aksv_write) {
> -		send_bytes += 5;
> -		val |= DP_AUX_CH_CTL_AUX_AKSV_SELECT;
> -	}
> -
> -	return val | intel_dp->get_aux_send_ctl(intel_dp,
> -						has_aux_irq,
> -						send_bytes,
> -						aux_clock_divider);
> -}
> -
>   static int
>   intel_dp_aux_xfer(struct intel_dp *intel_dp,
>   		  const uint8_t *send, int send_bytes,
> -		  uint8_t *recv, int recv_size, bool aksv_write)
> +		  uint8_t *recv, int recv_size,
> +		  u32 aux_send_ctl_flags)
>   {
>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   	struct drm_i915_private *dev_priv =
> @@ -1145,11 +1127,12 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>   	}
>   
>   	while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> -		u32 send_ctl = intel_dp_get_aux_send_ctl(intel_dp,
> -							 has_aux_irq,
> -							 send_bytes,
> -							 aux_clock_divider,
> -							 aksv_write);
> +		u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> +							  has_aux_irq,
> +							  send_bytes,
> +							  aux_clock_divider);
> +
> +		send_ctl |= aux_send_ctl_flags;
>   
>   		/* Must try at least 3 times according to DP spec */
>   		for (try = 0; try < 5; try++) {
> @@ -1287,7 +1270,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>   			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
>   
>   		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> -					rxbuf, rxsize, false);
> +					rxbuf, rxsize, 0);
>   		if (ret > 0) {
>   			msg->reply = rxbuf[0] >> 4;
>   
> @@ -1310,7 +1293,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>   			return -E2BIG;
>   
>   		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> -					rxbuf, rxsize, false);
> +					rxbuf, rxsize, 0);
>   		if (ret > 0) {
>   			msg->reply = rxbuf[0] >> 4;
>   			/*
> @@ -5085,7 +5068,7 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>   				u8 *an)
>   {
>   	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base);
> -	uint8_t txbuf[4], rxbuf[2], reply = 0;
> +	uint8_t txbuf[4+5] = {}, rxbuf[2], reply = 0;
You might want to use the macros for size of txbuf as  HEADER_SIZE + 
DRM_HDCP_KSV_LEN, as it is done in the next patch.
--Ram
>   	ssize_t dpcd_ret;
>   	int ret;
>   
> @@ -5110,7 +5093,8 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>   	txbuf[3] = DRM_HDCP_KSV_LEN - 1;
>   
>   	ret = intel_dp_aux_xfer(intel_dp, txbuf, sizeof(txbuf),
> -				rxbuf, sizeof(rxbuf), true);
> +				rxbuf, sizeof(rxbuf),
> +				DP_AUX_CH_CTL_AUX_AKSV_SELECT);
>   	if (ret < 0) {
>   		DRM_ERROR("Write Aksv over DP/AUX failed (%d)\n", ret);
>   		return ret;
Ville Syrjälä Feb. 23, 2018, 1:46 p.m. UTC | #2
On Fri, Feb 23, 2018 at 04:40:42PM +0530, Ramalingam C wrote:
> This is really making it cleaner.
> 
> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> 
> 
> 
> On Friday 23 February 2018 02:57 AM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Let's try to keep the details on the AKSV stuff concentrated
> > in one place. So move the control bit and +5 data size handling
> > there.
> >
> > v2: Increase txbuf[] to include the payload which intel_dp_aux_xfer()
> >      will still load into the registers even though the hardware
> >      will ignore it
> >
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: Ramalingam C <ramalingam.c@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_dp.c | 42 +++++++++++++----------------------------
> >   1 file changed, 13 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 217cc6aee477..0d699d230b77 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1059,29 +1059,11 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> >   	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> >   }
> >   
> > -static uint32_t intel_dp_get_aux_send_ctl(struct intel_dp *intel_dp,
> > -					  bool has_aux_irq,
> > -					  int send_bytes,
> > -					  uint32_t aux_clock_divider,
> > -					  bool aksv_write)
> > -{
> > -	uint32_t val = 0;
> > -
> > -	if (aksv_write) {
> > -		send_bytes += 5;
> > -		val |= DP_AUX_CH_CTL_AUX_AKSV_SELECT;
> > -	}
> > -
> > -	return val | intel_dp->get_aux_send_ctl(intel_dp,
> > -						has_aux_irq,
> > -						send_bytes,
> > -						aux_clock_divider);
> > -}
> > -
> >   static int
> >   intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >   		  const uint8_t *send, int send_bytes,
> > -		  uint8_t *recv, int recv_size, bool aksv_write)
> > +		  uint8_t *recv, int recv_size,
> > +		  u32 aux_send_ctl_flags)
> >   {
> >   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >   	struct drm_i915_private *dev_priv =
> > @@ -1145,11 +1127,12 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >   	}
> >   
> >   	while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> > -		u32 send_ctl = intel_dp_get_aux_send_ctl(intel_dp,
> > -							 has_aux_irq,
> > -							 send_bytes,
> > -							 aux_clock_divider,
> > -							 aksv_write);
> > +		u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> > +							  has_aux_irq,
> > +							  send_bytes,
> > +							  aux_clock_divider);
> > +
> > +		send_ctl |= aux_send_ctl_flags;
> >   
> >   		/* Must try at least 3 times according to DP spec */
> >   		for (try = 0; try < 5; try++) {
> > @@ -1287,7 +1270,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> >   			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
> >   
> >   		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> > -					rxbuf, rxsize, false);
> > +					rxbuf, rxsize, 0);
> >   		if (ret > 0) {
> >   			msg->reply = rxbuf[0] >> 4;
> >   
> > @@ -1310,7 +1293,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> >   			return -E2BIG;
> >   
> >   		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> > -					rxbuf, rxsize, false);
> > +					rxbuf, rxsize, 0);
> >   		if (ret > 0) {
> >   			msg->reply = rxbuf[0] >> 4;
> >   			/*
> > @@ -5085,7 +5068,7 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
> >   				u8 *an)
> >   {
> >   	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base);
> > -	uint8_t txbuf[4], rxbuf[2], reply = 0;
> > +	uint8_t txbuf[4+5] = {}, rxbuf[2], reply = 0;
> You might want to use the macros for size of txbuf as  HEADER_SIZE + 
> DRM_HDCP_KSV_LEN, as it is done in the next patch.

As the original code was using a bare 5 I figured I'll keep using it here
as well to make it easier to see what's moving where.

> --Ram
> >   	ssize_t dpcd_ret;
> >   	int ret;
> >   
> > @@ -5110,7 +5093,8 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
> >   	txbuf[3] = DRM_HDCP_KSV_LEN - 1;
> >   
> >   	ret = intel_dp_aux_xfer(intel_dp, txbuf, sizeof(txbuf),
> > -				rxbuf, sizeof(rxbuf), true);
> > +				rxbuf, sizeof(rxbuf),
> > +				DP_AUX_CH_CTL_AUX_AKSV_SELECT);
> >   	if (ret < 0) {
> >   		DRM_ERROR("Write Aksv over DP/AUX failed (%d)\n", ret);
> >   		return ret;
Ramalingam C Feb. 23, 2018, 2:44 p.m. UTC | #3
On Friday 23 February 2018 07:16 PM, Ville Syrjälä wrote:
> On Fri, Feb 23, 2018 at 04:40:42PM +0530, Ramalingam C wrote:
>> This is really making it cleaner.
>>
>> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
>>
>>
>>
>> On Friday 23 February 2018 02:57 AM, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Let's try to keep the details on the AKSV stuff concentrated
>>> in one place. So move the control bit and +5 data size handling
>>> there.
>>>
>>> v2: Increase txbuf[] to include the payload which intel_dp_aux_xfer()
>>>       will still load into the registers even though the hardware
>>>       will ignore it
>>>
>>> Cc: Sean Paul <seanpaul@chromium.org>
>>> Cc: Ramalingam C <ramalingam.c@intel.com>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_dp.c | 42 +++++++++++++----------------------------
>>>    1 file changed, 13 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 217cc6aee477..0d699d230b77 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -1059,29 +1059,11 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
>>>    	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
>>>    }
>>>    
>>> -static uint32_t intel_dp_get_aux_send_ctl(struct intel_dp *intel_dp,
>>> -					  bool has_aux_irq,
>>> -					  int send_bytes,
>>> -					  uint32_t aux_clock_divider,
>>> -					  bool aksv_write)
>>> -{
>>> -	uint32_t val = 0;
>>> -
>>> -	if (aksv_write) {
>>> -		send_bytes += 5;
>>> -		val |= DP_AUX_CH_CTL_AUX_AKSV_SELECT;
>>> -	}
>>> -
>>> -	return val | intel_dp->get_aux_send_ctl(intel_dp,
>>> -						has_aux_irq,
>>> -						send_bytes,
>>> -						aux_clock_divider);
>>> -}
>>> -
>>>    static int
>>>    intel_dp_aux_xfer(struct intel_dp *intel_dp,
>>>    		  const uint8_t *send, int send_bytes,
>>> -		  uint8_t *recv, int recv_size, bool aksv_write)
>>> +		  uint8_t *recv, int recv_size,
>>> +		  u32 aux_send_ctl_flags)
>>>    {
>>>    	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>>    	struct drm_i915_private *dev_priv =
>>> @@ -1145,11 +1127,12 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
>>>    	}
>>>    
>>>    	while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
>>> -		u32 send_ctl = intel_dp_get_aux_send_ctl(intel_dp,
>>> -							 has_aux_irq,
>>> -							 send_bytes,
>>> -							 aux_clock_divider,
>>> -							 aksv_write);
>>> +		u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
>>> +							  has_aux_irq,
>>> +							  send_bytes,
>>> +							  aux_clock_divider);
>>> +
>>> +		send_ctl |= aux_send_ctl_flags;
>>>    
>>>    		/* Must try at least 3 times according to DP spec */
>>>    		for (try = 0; try < 5; try++) {
>>> @@ -1287,7 +1270,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>>>    			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
>>>    
>>>    		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
>>> -					rxbuf, rxsize, false);
>>> +					rxbuf, rxsize, 0);
>>>    		if (ret > 0) {
>>>    			msg->reply = rxbuf[0] >> 4;
>>>    
>>> @@ -1310,7 +1293,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>>>    			return -E2BIG;
>>>    
>>>    		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
>>> -					rxbuf, rxsize, false);
>>> +					rxbuf, rxsize, 0);
>>>    		if (ret > 0) {
>>>    			msg->reply = rxbuf[0] >> 4;
>>>    			/*
>>> @@ -5085,7 +5068,7 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>>>    				u8 *an)
>>>    {
>>>    	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base);
>>> -	uint8_t txbuf[4], rxbuf[2], reply = 0;
>>> +	uint8_t txbuf[4+5] = {}, rxbuf[2], reply = 0;
>> You might want to use the macros for size of txbuf as  HEADER_SIZE +
>> DRM_HDCP_KSV_LEN, as it is done in the next patch.
> As the original code was using a bare 5 I figured I'll keep using it here
> as well to make it easier to see what's moving where.
Suggested the above, to make the 8th patch just for pulling out the aux 
header population.
I am happy with the current shape too :)

--Ram
>
>> --Ram
>>>    	ssize_t dpcd_ret;
>>>    	int ret;
>>>    
>>> @@ -5110,7 +5093,8 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
>>>    	txbuf[3] = DRM_HDCP_KSV_LEN - 1;
>>>    
>>>    	ret = intel_dp_aux_xfer(intel_dp, txbuf, sizeof(txbuf),
>>> -				rxbuf, sizeof(rxbuf), true);
>>> +				rxbuf, sizeof(rxbuf),
>>> +				DP_AUX_CH_CTL_AUX_AKSV_SELECT);
>>>    	if (ret < 0) {
>>>    		DRM_ERROR("Write Aksv over DP/AUX failed (%d)\n", ret);
>>>    		return ret;
Ville Syrjälä March 2, 2018, 4:30 p.m. UTC | #4
On Fri, Feb 23, 2018 at 08:14:53PM +0530, Ramalingam C wrote:
> 
> 
> On Friday 23 February 2018 07:16 PM, Ville Syrjälä wrote:
> > On Fri, Feb 23, 2018 at 04:40:42PM +0530, Ramalingam C wrote:
> >> This is really making it cleaner.
> >>
> >> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> >>
> >>
> >>
> >> On Friday 23 February 2018 02:57 AM, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> Let's try to keep the details on the AKSV stuff concentrated
> >>> in one place. So move the control bit and +5 data size handling
> >>> there.
> >>>
> >>> v2: Increase txbuf[] to include the payload which intel_dp_aux_xfer()
> >>>       will still load into the registers even though the hardware
> >>>       will ignore it
> >>>
> >>> Cc: Sean Paul <seanpaul@chromium.org>
> >>> Cc: Ramalingam C <ramalingam.c@intel.com>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/intel_dp.c | 42 +++++++++++++----------------------------
> >>>    1 file changed, 13 insertions(+), 29 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>> index 217cc6aee477..0d699d230b77 100644
> >>> --- a/drivers/gpu/drm/i915/intel_dp.c
> >>> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >>> @@ -1059,29 +1059,11 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
> >>>    	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> >>>    }
> >>>    
> >>> -static uint32_t intel_dp_get_aux_send_ctl(struct intel_dp *intel_dp,
> >>> -					  bool has_aux_irq,
> >>> -					  int send_bytes,
> >>> -					  uint32_t aux_clock_divider,
> >>> -					  bool aksv_write)
> >>> -{
> >>> -	uint32_t val = 0;
> >>> -
> >>> -	if (aksv_write) {
> >>> -		send_bytes += 5;
> >>> -		val |= DP_AUX_CH_CTL_AUX_AKSV_SELECT;
> >>> -	}
> >>> -
> >>> -	return val | intel_dp->get_aux_send_ctl(intel_dp,
> >>> -						has_aux_irq,
> >>> -						send_bytes,
> >>> -						aux_clock_divider);
> >>> -}
> >>> -
> >>>    static int
> >>>    intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >>>    		  const uint8_t *send, int send_bytes,
> >>> -		  uint8_t *recv, int recv_size, bool aksv_write)
> >>> +		  uint8_t *recv, int recv_size,
> >>> +		  u32 aux_send_ctl_flags)
> >>>    {
> >>>    	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>>    	struct drm_i915_private *dev_priv =
> >>> @@ -1145,11 +1127,12 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >>>    	}
> >>>    
> >>>    	while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
> >>> -		u32 send_ctl = intel_dp_get_aux_send_ctl(intel_dp,
> >>> -							 has_aux_irq,
> >>> -							 send_bytes,
> >>> -							 aux_clock_divider,
> >>> -							 aksv_write);
> >>> +		u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
> >>> +							  has_aux_irq,
> >>> +							  send_bytes,
> >>> +							  aux_clock_divider);
> >>> +
> >>> +		send_ctl |= aux_send_ctl_flags;
> >>>    
> >>>    		/* Must try at least 3 times according to DP spec */
> >>>    		for (try = 0; try < 5; try++) {
> >>> @@ -1287,7 +1270,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> >>>    			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
> >>>    
> >>>    		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> >>> -					rxbuf, rxsize, false);
> >>> +					rxbuf, rxsize, 0);
> >>>    		if (ret > 0) {
> >>>    			msg->reply = rxbuf[0] >> 4;
> >>>    
> >>> @@ -1310,7 +1293,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> >>>    			return -E2BIG;
> >>>    
> >>>    		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
> >>> -					rxbuf, rxsize, false);
> >>> +					rxbuf, rxsize, 0);
> >>>    		if (ret > 0) {
> >>>    			msg->reply = rxbuf[0] >> 4;
> >>>    			/*
> >>> @@ -5085,7 +5068,7 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
> >>>    				u8 *an)
> >>>    {
> >>>    	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base);
> >>> -	uint8_t txbuf[4], rxbuf[2], reply = 0;
> >>> +	uint8_t txbuf[4+5] = {}, rxbuf[2], reply = 0;
> >> You might want to use the macros for size of txbuf as  HEADER_SIZE +
> >> DRM_HDCP_KSV_LEN, as it is done in the next patch.
> > As the original code was using a bare 5 I figured I'll keep using it here
> > as well to make it easier to see what's moving where.
> Suggested the above, to make the 8th patch just for pulling out the aux 
> header population.
> I am happy with the current shape too :)

I decided to be lazy and leave it as is.

Remainder of the series pushed to dinq. Thanks for the reviews.

> 
> --Ram
> >
> >> --Ram
> >>>    	ssize_t dpcd_ret;
> >>>    	int ret;
> >>>    
> >>> @@ -5110,7 +5093,8 @@ int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
> >>>    	txbuf[3] = DRM_HDCP_KSV_LEN - 1;
> >>>    
> >>>    	ret = intel_dp_aux_xfer(intel_dp, txbuf, sizeof(txbuf),
> >>> -				rxbuf, sizeof(rxbuf), true);
> >>> +				rxbuf, sizeof(rxbuf),
> >>> +				DP_AUX_CH_CTL_AUX_AKSV_SELECT);
> >>>    	if (ret < 0) {
> >>>    		DRM_ERROR("Write Aksv over DP/AUX failed (%d)\n", ret);
> >>>    		return ret;
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 217cc6aee477..0d699d230b77 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1059,29 +1059,11 @@  static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
 	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
 }
 
-static uint32_t intel_dp_get_aux_send_ctl(struct intel_dp *intel_dp,
-					  bool has_aux_irq,
-					  int send_bytes,
-					  uint32_t aux_clock_divider,
-					  bool aksv_write)
-{
-	uint32_t val = 0;
-
-	if (aksv_write) {
-		send_bytes += 5;
-		val |= DP_AUX_CH_CTL_AUX_AKSV_SELECT;
-	}
-
-	return val | intel_dp->get_aux_send_ctl(intel_dp,
-						has_aux_irq,
-						send_bytes,
-						aux_clock_divider);
-}
-
 static int
 intel_dp_aux_xfer(struct intel_dp *intel_dp,
 		  const uint8_t *send, int send_bytes,
-		  uint8_t *recv, int recv_size, bool aksv_write)
+		  uint8_t *recv, int recv_size,
+		  u32 aux_send_ctl_flags)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv =
@@ -1145,11 +1127,12 @@  intel_dp_aux_xfer(struct intel_dp *intel_dp,
 	}
 
 	while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) {
-		u32 send_ctl = intel_dp_get_aux_send_ctl(intel_dp,
-							 has_aux_irq,
-							 send_bytes,
-							 aux_clock_divider,
-							 aksv_write);
+		u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp,
+							  has_aux_irq,
+							  send_bytes,
+							  aux_clock_divider);
+
+		send_ctl |= aux_send_ctl_flags;
 
 		/* Must try at least 3 times according to DP spec */
 		for (try = 0; try < 5; try++) {
@@ -1287,7 +1270,7 @@  intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size);
 
 		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
-					rxbuf, rxsize, false);
+					rxbuf, rxsize, 0);
 		if (ret > 0) {
 			msg->reply = rxbuf[0] >> 4;
 
@@ -1310,7 +1293,7 @@  intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			return -E2BIG;
 
 		ret = intel_dp_aux_xfer(intel_dp, txbuf, txsize,
-					rxbuf, rxsize, false);
+					rxbuf, rxsize, 0);
 		if (ret > 0) {
 			msg->reply = rxbuf[0] >> 4;
 			/*
@@ -5085,7 +5068,7 @@  int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 				u8 *an)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base);
-	uint8_t txbuf[4], rxbuf[2], reply = 0;
+	uint8_t txbuf[4+5] = {}, rxbuf[2], reply = 0;
 	ssize_t dpcd_ret;
 	int ret;
 
@@ -5110,7 +5093,8 @@  int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port,
 	txbuf[3] = DRM_HDCP_KSV_LEN - 1;
 
 	ret = intel_dp_aux_xfer(intel_dp, txbuf, sizeof(txbuf),
-				rxbuf, sizeof(rxbuf), true);
+				rxbuf, sizeof(rxbuf),
+				DP_AUX_CH_CTL_AUX_AKSV_SELECT);
 	if (ret < 0) {
 		DRM_ERROR("Write Aksv over DP/AUX failed (%d)\n", ret);
 		return ret;