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@xxxxxxxxx>
> >>
> >>
> >>
> >> On Friday 23 February 2018 02:57 AM, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>>
> >>> 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@xxxxxxxxxxxx>
> >>> Cc: Ramalingam C <ramalingam.c@xxxxxxxxx>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>> ---
> >>> 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;
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
