| Message ID | 20230608093218.511280-1-arun.r.murthy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCHv2] drm/i915/display/dp: On AUX xfer timeout restart freshly | expand |
On Thu, Jun 08, 2023 at 03:02:18PM +0530, Arun R Murthy wrote: > At the begining of the aux transfer a check for aux control busy bit is > done. Then as per the spec on aux transfer timeout, need to retry > freshly for 3 times with a delay which is taken care by the control > register. > On each of these 3 trials a check for busy has to be done so as to start > freshly. > > v2: updated the commit message > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 +++++++++------------ > 1 file changed, 22 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c > index 197c6e81db14..25090542dd9f 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > @@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > * it using the same AUX CH simultaneously > */ > > - /* Try to wait for any previous AUX channel activity */ > - for (try = 0; try < 3; try++) { > - status = intel_de_read_notrace(i915, ch_ctl); > - if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0) > - break; > - msleep(1); > - } > - /* just trace the final value */ > - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > - > - if (try == 3) { > - const u32 status = intel_de_read(i915, ch_ctl); > - > - if (status != intel_dp->aux_busy_last_status) { > - drm_WARN(&i915->drm, 1, > - "%s: not started (status 0x%08x)\n", > - intel_dp->aux.name, status); > - intel_dp->aux_busy_last_status = status; > - } > - > - ret = -EBUSY; > - goto out; > - } > - > /* Only 5 data registers! */ > if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) { > ret = -E2BIG; > @@ -304,14 +280,31 @@ 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, > + /* Must try at least 3 times according to DP spec */ > + for (try = 0; try < 5; try++) { > + u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > send_bytes, > aux_clock_divider); > > - send_ctl |= aux_send_ctl_flags; > + send_ctl |= aux_send_ctl_flags; Why is send_ctl recomputed in each iteration? > + > + /* Try to wait for any previous AUX channel activity */ > + status = intel_dp_aux_wait_done(intel_dp); How does it help to wait here for the same condition that was already waited for at the end of the loop? > + /* just trace the final value */ > + trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > + > + if (status & DP_AUX_CH_CTL_SEND_BUSY) { > + drm_WARN(&i915->drm, 1, > + "%s: not started, previous Tx still in process (status 0x%08x)\n", > + intel_dp->aux.name, status); > + intel_dp->aux_busy_last_status = status; > + if (try > 3) { > + ret = -EBUSY; > + goto out; > + } else > + continue; > + } > > - /* Must try at least 3 times according to DP spec */ > - for (try = 0; try < 5; try++) { > /* Load the send data into the aux channel data registers */ > for (i = 0; i < send_bytes; i += 4) > intel_de_write(i915, ch_data[i >> 2], > @@ -321,6 +314,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > /* Send the command and wait for it to complete */ > intel_de_write(i915, ch_ctl, send_ctl); > > + /* TODO: if typeC then 4.2ms else 800us. For DG2 add 1.5ms for both cases */ > status = intel_dp_aux_wait_done(intel_dp); > > /* Clear done status and any errors */ > -- > 2.25.1 >
> -----Original Message----- > From: Deak, Imre <imre.deak@intel.com> > Sent: Monday, June 12, 2023 5:07 PM > To: Murthy, Arun R <arun.r.murthy@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout > restart freshly > > On Thu, Jun 08, 2023 at 03:02:18PM +0530, Arun R Murthy wrote: > > At the begining of the aux transfer a check for aux control busy bit > > is done. Then as per the spec on aux transfer timeout, need to retry > > freshly for 3 times with a delay which is taken care by the control > > register. > > On each of these 3 trials a check for busy has to be done so as to > > start freshly. > > > > v2: updated the commit message > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 > > +++++++++------------ > > 1 file changed, 22 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > index 197c6e81db14..25090542dd9f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > @@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > > * it using the same AUX CH simultaneously > > */ > > > > - /* Try to wait for any previous AUX channel activity */ > > - for (try = 0; try < 3; try++) { > > - status = intel_de_read_notrace(i915, ch_ctl); > > - if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0) > > - break; > > - msleep(1); > > - } > > - /* just trace the final value */ > > - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > > - > > - if (try == 3) { > > - const u32 status = intel_de_read(i915, ch_ctl); > > - > > - if (status != intel_dp->aux_busy_last_status) { > > - drm_WARN(&i915->drm, 1, > > - "%s: not started (status 0x%08x)\n", > > - intel_dp->aux.name, status); > > - intel_dp->aux_busy_last_status = status; > > - } > > - > > - ret = -EBUSY; > > - goto out; > > - } > > - > > /* Only 5 data registers! */ > > if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) { > > ret = -E2BIG; > > @@ -304,14 +280,31 @@ 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, > > + /* Must try at least 3 times according to DP spec */ > > + for (try = 0; try < 5; try++) { > > + u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > > send_bytes, > > aux_clock_divider); > > > > - send_ctl |= aux_send_ctl_flags; > > + send_ctl |= aux_send_ctl_flags; > > Why is send_ctl recomputed in each iteration? This can be moved outside the loop, since the value doesn't tend to change. > > > + > > + /* Try to wait for any previous AUX channel activity > */ > > + status = intel_dp_aux_wait_done(intel_dp); > > How does it help to wait here for the same condition that was already waited > for at the end of the loop? Here we are checking for busy bit so as to ensure that the previous transfer is complete and only then the new transfer is initiated. In the latter case, we sent the data and then wait to get the acknowledgement(busy/error/timeout). Check for busy is to be done before sending the data. Here it was done before this loop. The spec limits the trials for sending data to 3 in case of failure and in each of this iteration it has to be started freshly. So we need to ensure that the previous transfer is completed before sending the new data. Thanks and Regards, Arun R Murthy -------------------- > > > + /* just trace the final value */ > > + trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), > true); > > + > > + if (status & DP_AUX_CH_CTL_SEND_BUSY) { > > + drm_WARN(&i915->drm, 1, > > + "%s: not started, previous Tx still in > process (status 0x%08x)\n", > > + intel_dp->aux.name, status); > > + intel_dp->aux_busy_last_status = status; > > + if (try > 3) { > > + ret = -EBUSY; > > + goto out; > > + } else > > + continue; > > + } > > > > - /* Must try at least 3 times according to DP spec */ > > - for (try = 0; try < 5; try++) { > > /* Load the send data into the aux channel data > registers */ > > for (i = 0; i < send_bytes; i += 4) > > intel_de_write(i915, ch_data[i >> 2], @@ - > 321,6 +314,7 @@ > > intel_dp_aux_xfer(struct intel_dp *intel_dp, > > /* Send the command and wait for it to complete */ > > intel_de_write(i915, ch_ctl, send_ctl); > > > > + /* TODO: if typeC then 4.2ms else 800us. For DG2 > add 1.5ms for > > +both cases */ > > status = intel_dp_aux_wait_done(intel_dp); > > > > /* Clear done status and any errors */ > > -- > > 2.25.1 > >
On Mon, Jun 12, 2023 at 02:47:37PM +0300, Murthy, Arun R wrote: > > -----Original Message----- > > From: Deak, Imre <imre.deak@intel.com> > > Sent: Monday, June 12, 2023 5:07 PM > > To: Murthy, Arun R <arun.r.murthy@intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout > > restart freshly > > > > On Thu, Jun 08, 2023 at 03:02:18PM +0530, Arun R Murthy wrote: > > > At the begining of the aux transfer a check for aux control busy bit > > > is done. Then as per the spec on aux transfer timeout, need to retry > > > freshly for 3 times with a delay which is taken care by the control > > > register. > > > On each of these 3 trials a check for busy has to be done so as to > > > start freshly. > > > > > > v2: updated the commit message > > > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 > > > +++++++++------------ > > > 1 file changed, 22 insertions(+), 28 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > index 197c6e81db14..25090542dd9f 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > @@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > * it using the same AUX CH simultaneously > > > */ > > > > > > - /* Try to wait for any previous AUX channel activity */ > > > - for (try = 0; try < 3; try++) { > > > - status = intel_de_read_notrace(i915, ch_ctl); > > > - if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0) > > > - break; > > > - msleep(1); > > > - } > > > - /* just trace the final value */ > > > - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > > > - > > > - if (try == 3) { > > > - const u32 status = intel_de_read(i915, ch_ctl); > > > - > > > - if (status != intel_dp->aux_busy_last_status) { > > > - drm_WARN(&i915->drm, 1, > > > - "%s: not started (status 0x%08x)\n", > > > - intel_dp->aux.name, status); > > > - intel_dp->aux_busy_last_status = status; > > > - } > > > - > > > - ret = -EBUSY; > > > - goto out; > > > - } > > > - > > > /* Only 5 data registers! */ > > > if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) { > > > ret = -E2BIG; > > > @@ -304,14 +280,31 @@ 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, > > > + /* Must try at least 3 times according to DP spec */ > > > + for (try = 0; try < 5; try++) { > > > + u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > > > send_bytes, > > > aux_clock_divider); > > > > > > - send_ctl |= aux_send_ctl_flags; > > > + send_ctl |= aux_send_ctl_flags; > > > > Why is send_ctl recomputed in each iteration? > > This can be moved outside the loop, since the value doesn't tend to change. > > > > > > + > > > + /* Try to wait for any previous AUX channel activity > > */ > > > + status = intel_dp_aux_wait_done(intel_dp); > > > > How does it help to wait here for the same condition that was already waited > > for at the end of the loop? > > Here we are checking for busy bit so as to ensure that the previous > transfer is complete and only then the new transfer is initiated. > > In the latter case, we sent the data and then wait to get the > acknowledgement(busy/error/timeout). > > Check for busy is to be done before sending the data. Here it was done > before this loop. The spec limits the trials for sending data to 3 in > case of failure and in each of this iteration it has to be started > freshly. So we need to ensure that the previous transfer is completed > before sending the new data. Not sure what you mean by "freshly". One potential problem in the current code I can see is that if BUSY is still set after intel_dp_aux_wait_done() returns none of the control register fields should be changed, so I think at that point the transfer should be aborted. Also since a while back intel_dp_aux_wait_done() was changed to poll for the transfer completion instead of waiting for an interrupt, the corresponding interrupt should not be enabled either in the control register. > Thanks and Regards, > Arun R Murthy > -------------------- > > > > > + /* just trace the final value */ > > > + trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), > > true); > > > + > > > + if (status & DP_AUX_CH_CTL_SEND_BUSY) { > > > + drm_WARN(&i915->drm, 1, > > > + "%s: not started, previous Tx still in > > process (status 0x%08x)\n", > > > + intel_dp->aux.name, status); > > > + intel_dp->aux_busy_last_status = status; > > > + if (try > 3) { > > > + ret = -EBUSY; > > > + goto out; > > > + } else > > > + continue; > > > + } > > > > > > - /* Must try at least 3 times according to DP spec */ > > > - for (try = 0; try < 5; try++) { > > > /* Load the send data into the aux channel data > > registers */ > > > for (i = 0; i < send_bytes; i += 4) > > > intel_de_write(i915, ch_data[i >> 2], @@ - > > 321,6 +314,7 @@ > > > intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > /* Send the command and wait for it to complete */ > > > intel_de_write(i915, ch_ctl, send_ctl); > > > > > > + /* TODO: if typeC then 4.2ms else 800us. For DG2 > > add 1.5ms for > > > +both cases */ > > > status = intel_dp_aux_wait_done(intel_dp); > > > > > > /* Clear done status and any errors */ > > > -- > > > 2.25.1 > > >
> -----Original Message----- > From: Deak, Imre <imre.deak@intel.com> > Sent: Monday, June 12, 2023 6:28 PM > To: Murthy, Arun R <arun.r.murthy@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer timeout > restart freshly > > On Mon, Jun 12, 2023 at 02:47:37PM +0300, Murthy, Arun R wrote: > > > -----Original Message----- > > > From: Deak, Imre <imre.deak@intel.com> > > > Sent: Monday, June 12, 2023 5:07 PM > > > To: Murthy, Arun R <arun.r.murthy@intel.com> > > > Cc: intel-gfx@lists.freedesktop.org > > > Subject: Re: [Intel-gfx] [PATCHv2] drm/i915/display/dp: On AUX xfer > > > timeout restart freshly > > > > > > On Thu, Jun 08, 2023 at 03:02:18PM +0530, Arun R Murthy wrote: > > > > At the begining of the aux transfer a check for aux control busy > > > > bit is done. Then as per the spec on aux transfer timeout, need to > > > > retry freshly for 3 times with a delay which is taken care by the > > > > control register. > > > > On each of these 3 trials a check for busy has to be done so as to > > > > start freshly. > > > > > > > > v2: updated the commit message > > > > > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 > > > > +++++++++------------ > > > > 1 file changed, 22 insertions(+), 28 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > > b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > > index 197c6e81db14..25090542dd9f 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c > > > > @@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > > * it using the same AUX CH simultaneously > > > > */ > > > > > > > > - /* Try to wait for any previous AUX channel activity */ > > > > - for (try = 0; try < 3; try++) { > > > > - status = intel_de_read_notrace(i915, ch_ctl); > > > > - if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0) > > > > - break; > > > > - msleep(1); > > > > - } > > > > - /* just trace the final value */ > > > > - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); > > > > - > > > > - if (try == 3) { > > > > - const u32 status = intel_de_read(i915, ch_ctl); > > > > - > > > > - if (status != intel_dp->aux_busy_last_status) { > > > > - drm_WARN(&i915->drm, 1, > > > > - "%s: not started (status 0x%08x)\n", > > > > - intel_dp->aux.name, status); > > > > - intel_dp->aux_busy_last_status = status; > > > > - } > > > > - > > > > - ret = -EBUSY; > > > > - goto out; > > > > - } > > > > - > > > > /* Only 5 data registers! */ > > > > if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) { > > > > ret = -E2BIG; > > > > @@ -304,14 +280,31 @@ 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, > > > > + /* Must try at least 3 times according to DP spec */ > > > > + for (try = 0; try < 5; try++) { > > > > + u32 send_ctl = > > > > + intel_dp->get_aux_send_ctl(intel_dp, > > > > send_bytes, > > > > > > > > aux_clock_divider); > > > > > > > > - send_ctl |= aux_send_ctl_flags; > > > > + send_ctl |= aux_send_ctl_flags; > > > > > > Why is send_ctl recomputed in each iteration? > > > > This can be moved outside the loop, since the value doesn't tend to > change. > > > > > > > > > + > > > > + /* Try to wait for any previous AUX channel > > > > + activity > > > */ > > > > + status = intel_dp_aux_wait_done(intel_dp); > > > > > > How does it help to wait here for the same condition that was > > > already waited for at the end of the loop? > > > > Here we are checking for busy bit so as to ensure that the previous > > transfer is complete and only then the new transfer is initiated. > > > > In the latter case, we sent the data and then wait to get the > > acknowledgement(busy/error/timeout). > > > > Check for busy is to be done before sending the data. Here it was done > > before this loop. The spec limits the trials for sending data to 3 in > > case of failure and in each of this iteration it has to be started > > freshly. So we need to ensure that the previous transfer is completed > > before sending the new data. > > Not sure what you mean by "freshly". One potential problem in the current > code I can see is that if BUSY is still set after > intel_dp_aux_wait_done() returns none of the control register fields should > be changed, so I think at that point the transfer should be aborted. Good Catch! I missed that. Will surely take care of this in the next version. > > Also since a while back intel_dp_aux_wait_done() was changed to poll for > the transfer completion instead of waiting for an interrupt, the > corresponding interrupt should not be enabled either in the control register. Yes this is in my TODO list, most likely in my next patch. Thanks and Regards, Arun R Murthy -------------------- > > > Thanks and Regards, > > Arun R Murthy > > -------------------- > > > > > > > + /* just trace the final value */ > > > > + trace_i915_reg_rw(false, ch_ctl, status, > > > > + sizeof(status), > > > true); > > > > + > > > > + if (status & DP_AUX_CH_CTL_SEND_BUSY) { > > > > + drm_WARN(&i915->drm, 1, > > > > + "%s: not started, previous Tx > > > > + still in > > > process (status 0x%08x)\n", > > > > + intel_dp->aux.name, status); > > > > + intel_dp->aux_busy_last_status = status; > > > > + if (try > 3) { > > > > + ret = -EBUSY; > > > > + goto out; > > > > + } else > > > > + continue; > > > > + } > > > > > > > > - /* Must try at least 3 times according to DP spec */ > > > > - for (try = 0; try < 5; try++) { > > > > /* Load the send data into the aux channel > > > > data > > > registers */ > > > > for (i = 0; i < send_bytes; i += 4) > > > > intel_de_write(i915, ch_data[i >> 2], > > > > @@ - > > > 321,6 +314,7 @@ > > > > intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > > /* Send the command and wait for it to complete */ > > > > intel_de_write(i915, ch_ctl, send_ctl); > > > > > > > > + /* TODO: if typeC then 4.2ms else 800us. For > > > > + DG2 > > > add 1.5ms for > > > > +both cases */ > > > > status = intel_dp_aux_wait_done(intel_dp); > > > > > > > > /* Clear done status and any errors */ > > > > -- > > > > 2.25.1 > > > >
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux.c b/drivers/gpu/drm/i915/display/intel_dp_aux.c index 197c6e81db14..25090542dd9f 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux.c @@ -273,30 +273,6 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, * it using the same AUX CH simultaneously */ - /* Try to wait for any previous AUX channel activity */ - for (try = 0; try < 3; try++) { - status = intel_de_read_notrace(i915, ch_ctl); - if ((status & DP_AUX_CH_CTL_SEND_BUSY) == 0) - break; - msleep(1); - } - /* just trace the final value */ - trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); - - if (try == 3) { - const u32 status = intel_de_read(i915, ch_ctl); - - if (status != intel_dp->aux_busy_last_status) { - drm_WARN(&i915->drm, 1, - "%s: not started (status 0x%08x)\n", - intel_dp->aux.name, status); - intel_dp->aux_busy_last_status = status; - } - - ret = -EBUSY; - goto out; - } - /* Only 5 data registers! */ if (drm_WARN_ON(&i915->drm, send_bytes > 20 || recv_size > 20)) { ret = -E2BIG; @@ -304,14 +280,31 @@ 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, + /* Must try at least 3 times according to DP spec */ + for (try = 0; try < 5; try++) { + u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, send_bytes, aux_clock_divider); - send_ctl |= aux_send_ctl_flags; + send_ctl |= aux_send_ctl_flags; + + /* Try to wait for any previous AUX channel activity */ + status = intel_dp_aux_wait_done(intel_dp); + /* just trace the final value */ + trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true); + + if (status & DP_AUX_CH_CTL_SEND_BUSY) { + drm_WARN(&i915->drm, 1, + "%s: not started, previous Tx still in process (status 0x%08x)\n", + intel_dp->aux.name, status); + intel_dp->aux_busy_last_status = status; + if (try > 3) { + ret = -EBUSY; + goto out; + } else + continue; + } - /* Must try at least 3 times according to DP spec */ - for (try = 0; try < 5; try++) { /* Load the send data into the aux channel data registers */ for (i = 0; i < send_bytes; i += 4) intel_de_write(i915, ch_data[i >> 2], @@ -321,6 +314,7 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp, /* Send the command and wait for it to complete */ intel_de_write(i915, ch_ctl, send_ctl); + /* TODO: if typeC then 4.2ms else 800us. For DG2 add 1.5ms for both cases */ status = intel_dp_aux_wait_done(intel_dp); /* Clear done status and any errors */
At the begining of the aux transfer a check for aux control busy bit is done. Then as per the spec on aux transfer timeout, need to retry freshly for 3 times with a delay which is taken care by the control register. On each of these 3 trials a check for busy has to be done so as to start freshly. v2: updated the commit message Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> --- drivers/gpu/drm/i915/display/intel_dp_aux.c | 50 +++++++++------------ 1 file changed, 22 insertions(+), 28 deletions(-)