From: Xinlei Lee <[email protected]>
DP 1.4a Section 2.8.7.1.5.6.1:
A DP Source device shall retry at least seven times upon receiving
AUX_DEFER before giving up the AUX transaction.
The drm_dp_i2c_do_msg() function in the drm_dp_helper.c file will
judge the status of the msg->reply parameter passed to aux_transfer
ange-the-aux-retries-times-when-re.patchfor different processing.
Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
Signed-off-by: Xinlei Lee <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dp.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index 1f94fcc144d3..767b71da31a4 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -806,10 +806,9 @@ static int mtk_dp_aux_wait_for_completion(struct mtk_dp *mtk_dp, bool is_read)
}
static int mtk_dp_aux_do_transfer(struct mtk_dp *mtk_dp, bool is_read, u8 cmd,
- u32 addr, u8 *buf, size_t length)
+ u32 addr, u8 *buf, size_t length, u8 *reply_cmd)
{
int ret;
- u32 reply_cmd;
if (is_read && (length > DP_AUX_MAX_PAYLOAD_BYTES ||
(cmd == DP_AUX_NATIVE_READ && !length)))
@@ -841,10 +840,10 @@ static int mtk_dp_aux_do_transfer(struct mtk_dp *mtk_dp, bool is_read, u8 cmd,
/* Wait for feedback from sink device. */
ret = mtk_dp_aux_wait_for_completion(mtk_dp, is_read);
- reply_cmd = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3624) &
- AUX_RX_REPLY_COMMAND_AUX_TX_P0_MASK;
+ *reply_cmd = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3624) &
+ AUX_RX_REPLY_COMMAND_AUX_TX_P0_MASK;
- if (ret || reply_cmd) {
+ if (ret) {
u32 phy_status = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3628) &
AUX_RX_PHY_STATE_AUX_TX_P0_MASK;
if (phy_status != AUX_RX_PHY_STATE_AUX_TX_P0_RX_IDLE) {
@@ -2070,7 +2069,7 @@ static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux,
ret = mtk_dp_aux_do_transfer(mtk_dp, is_read, request,
msg->address + accessed_bytes,
msg->buffer + accessed_bytes,
- to_access);
+ to_access, &msg->reply);
if (ret) {
drm_info(mtk_dp->drm_dev,
@@ -2080,7 +2079,6 @@ static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux,
accessed_bytes += to_access;
} while (accessed_bytes < msg->size);
- msg->reply = DP_AUX_NATIVE_REPLY_ACK | DP_AUX_I2C_REPLY_ACK;
return msg->size;
err:
msg->reply = DP_AUX_NATIVE_REPLY_NACK | DP_AUX_I2C_REPLY_NACK;
--
2.18.0
Hi, Xinlei:
<[email protected]> 於 2023年3月29日 週三 下午2:43寫道：
>
> From: Xinlei Lee <[email protected]>
>
> DP 1.4a Section 2.8.7.1.5.6.1:
> A DP Source device shall retry at least seven times upon receiving
> AUX_DEFER before giving up the AUX transaction.
>
> The drm_dp_i2c_do_msg() function in the drm_dp_helper.c file will
> judge the status of the msg->reply parameter passed to aux_transfer
> ange-the-aux-retries-times-when-re.patchfor different processing.
>
> Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort driver")
> Signed-off-by: Xinlei Lee <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dp.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index 1f94fcc144d3..767b71da31a4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -806,10 +806,9 @@ static int mtk_dp_aux_wait_for_completion(struct mtk_dp *mtk_dp, bool is_read)
> }
>
> static int mtk_dp_aux_do_transfer(struct mtk_dp *mtk_dp, bool is_read, u8 cmd,
> - u32 addr, u8 *buf, size_t length)
> + u32 addr, u8 *buf, size_t length, u8 *reply_cmd)
> {
> int ret;
> - u32 reply_cmd;
>
> if (is_read && (length > DP_AUX_MAX_PAYLOAD_BYTES ||
> (cmd == DP_AUX_NATIVE_READ && !length)))
> @@ -841,10 +840,10 @@ static int mtk_dp_aux_do_transfer(struct mtk_dp *mtk_dp, bool is_read, u8 cmd,
> /* Wait for feedback from sink device. */
> ret = mtk_dp_aux_wait_for_completion(mtk_dp, is_read);
>
> - reply_cmd = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3624) &
> - AUX_RX_REPLY_COMMAND_AUX_TX_P0_MASK;
> + *reply_cmd = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3624) &
> + AUX_RX_REPLY_COMMAND_AUX_TX_P0_MASK;
>
> - if (ret || reply_cmd) {
> + if (ret) {
> u32 phy_status = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3628) &
> AUX_RX_PHY_STATE_AUX_TX_P0_MASK;
> if (phy_status != AUX_RX_PHY_STATE_AUX_TX_P0_RX_IDLE) {
> @@ -2070,7 +2069,7 @@ static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux,
> ret = mtk_dp_aux_do_transfer(mtk_dp, is_read, request,
> msg->address + accessed_bytes,
> msg->buffer + accessed_bytes,
> - to_access);
> + to_access, &msg->reply);
>
> if (ret) {
> drm_info(mtk_dp->drm_dev,
> @@ -2080,7 +2079,6 @@ static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux,
> accessed_bytes += to_access;
> } while (accessed_bytes < msg->size);
>
> - msg->reply = DP_AUX_NATIVE_REPLY_ACK | DP_AUX_I2C_REPLY_ACK;
In your description, you just mention the retry count is 7 times, but
you does not mention you should change the reply. Why you modify this?
And where is the 7 times retry?
Regards,
Chun-Kuang.
> return msg->size;
> err:
> msg->reply = DP_AUX_NATIVE_REPLY_NACK | DP_AUX_I2C_REPLY_NACK;
> --
> 2.18.0
>
On Mon, 2023-04-03 at 11:49 +0800, Chun-Kuang Hu wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Hi, Xinlei:
>
> <[email protected]> 於 2023年3月29日 週三 下午2:43寫道：
> >
> > From: Xinlei Lee <[email protected]>
> >
> > DP 1.4a Section 2.8.7.1.5.6.1:
> > A DP Source device shall retry at least seven times upon receiving
> > AUX_DEFER before giving up the AUX transaction.
> >
> > The drm_dp_i2c_do_msg() function in the drm_dp_helper.c file will
> > judge the status of the msg->reply parameter passed to aux_transfer
> > ange-the-aux-retries-times-when-re.patchfor different processing.
> >
> > Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort
> > driver")
> > Signed-off-by: Xinlei Lee <[email protected]>
> > ---
> > drivers/gpu/drm/mediatek/mtk_dp.c | 12 +++++-------
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> > b/drivers/gpu/drm/mediatek/mtk_dp.c
> > index 1f94fcc144d3..767b71da31a4 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> > @@ -806,10 +806,9 @@ static int
> > mtk_dp_aux_wait_for_completion(struct mtk_dp *mtk_dp, bool is_read)
> > }
> >
> > static int mtk_dp_aux_do_transfer(struct mtk_dp *mtk_dp, bool
> > is_read, u8 cmd,
> > - u32 addr, u8 *buf, size_t length)
> > + u32 addr, u8 *buf, size_t length,
> > u8 *reply_cmd)
> > {
> > int ret;
> > - u32 reply_cmd;
> >
> > if (is_read && (length > DP_AUX_MAX_PAYLOAD_BYTES ||
> > (cmd == DP_AUX_NATIVE_READ && !length)))
> > @@ -841,10 +840,10 @@ static int mtk_dp_aux_do_transfer(struct
> > mtk_dp *mtk_dp, bool is_read, u8 cmd,
> > /* Wait for feedback from sink device. */
> > ret = mtk_dp_aux_wait_for_completion(mtk_dp, is_read);
> >
> > - reply_cmd = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3624) &
> > - AUX_RX_REPLY_COMMAND_AUX_TX_P0_MASK;
> > + *reply_cmd = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3624) &
> > + AUX_RX_REPLY_COMMAND_AUX_TX_P0_MASK;
> >
> > - if (ret || reply_cmd) {
> > + if (ret) {
> > u32 phy_status = mtk_dp_read(mtk_dp,
> > MTK_DP_AUX_P0_3628) &
> > AUX_RX_PHY_STATE_AUX_TX_P0_MASK;
> > if (phy_status !=
> > AUX_RX_PHY_STATE_AUX_TX_P0_RX_IDLE) {
> > @@ -2070,7 +2069,7 @@ static ssize_t mtk_dp_aux_transfer(struct
> > drm_dp_aux *mtk_aux,
> > ret = mtk_dp_aux_do_transfer(mtk_dp, is_read,
> > request,
> > msg->address +
> > accessed_bytes,
> > msg->buffer +
> > accessed_bytes,
> > - to_access);
> > + to_access, &msg-
> > >reply);
> >
> > if (ret) {
> > drm_info(mtk_dp->drm_dev,
> > @@ -2080,7 +2079,6 @@ static ssize_t mtk_dp_aux_transfer(struct
> > drm_dp_aux *mtk_aux,
> > accessed_bytes += to_access;
> > } while (accessed_bytes < msg->size);
> >
> > - msg->reply = DP_AUX_NATIVE_REPLY_ACK |
> > DP_AUX_I2C_REPLY_ACK;
>
> In your description, you just mention the retry count is 7 times, but
> you does not mention you should change the reply. Why you modify
> this?
> And where is the 7 times retry?
>
> Regards,
> Chun-Kuang.
>
> > return msg->size;
> > err:
> > msg->reply = DP_AUX_NATIVE_REPLY_NACK |
> > DP_AUX_I2C_REPLY_NACK;
> > --
> > 2.18.0
> >
Hi CK:
Thanks for your review!
This patch is to fix some DP sinks that return AUX_DEFER, and the dp
driver does not handle it according to the specification. DP_v1.4a
spec 2.8.1.2 describes that if the sink returns AUX_DEFER, DPTX may
retry later:
The logic before the modification is that reply_cmd returns ETIMEDOUT
if it is not AUX_ACK after the read operation, without considering the
retry operation when returning AUX_DEFER;
The modified logic is to add parameters to mtk_dp_aux_do_transfer() to
store the return value of the sink. In the dmr_dp_helper.c file,
drm_dp_i2c_do_msg calls aux->transfer and then performs retry
operation according to msg->reply. The 7 times specified in the spec
are also in this function defined in (max_retries).
Best Regards!
xinlei
Hi, Xinlei:
Xinlei Lee (李昕磊) <[email protected]> 於 2023年4月3日 週一 下午5:18寫道：
>
> On Mon, 2023-04-03 at 11:49 +0800, Chun-Kuang Hu wrote:
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >
> >
> > Hi, Xinlei:
> >
> > <[email protected]> 於 2023年3月29日 週三 下午2:43寫道：
> > >
> > > From: Xinlei Lee <[email protected]>
> > >
> > > DP 1.4a Section 2.8.7.1.5.6.1:
> > > A DP Source device shall retry at least seven times upon receiving
> > > AUX_DEFER before giving up the AUX transaction.
> > >
> > > The drm_dp_i2c_do_msg() function in the drm_dp_helper.c file will
> > > judge the status of the msg->reply parameter passed to aux_transfer
> > > ange-the-aux-retries-times-when-re.patchfor different processing.
> > >
> > > Fixes: f70ac097a2cf ("drm/mediatek: Add MT8195 Embedded DisplayPort
> > > driver")
> > > Signed-off-by: Xinlei Lee <[email protected]>
> > > ---
> > > drivers/gpu/drm/mediatek/mtk_dp.c | 12 +++++-------
> > > 1 file changed, 5 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> > > b/drivers/gpu/drm/mediatek/mtk_dp.c
> > > index 1f94fcc144d3..767b71da31a4 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> > > @@ -806,10 +806,9 @@ static int
> > > mtk_dp_aux_wait_for_completion(struct mtk_dp *mtk_dp, bool is_read)
> > > }
> > >
> > > static int mtk_dp_aux_do_transfer(struct mtk_dp *mtk_dp, bool
> > > is_read, u8 cmd,
> > > - u32 addr, u8 *buf, size_t length)
> > > + u32 addr, u8 *buf, size_t length,
> > > u8 *reply_cmd)
> > > {
> > > int ret;
> > > - u32 reply_cmd;
> > >
> > > if (is_read && (length > DP_AUX_MAX_PAYLOAD_BYTES ||
> > > (cmd == DP_AUX_NATIVE_READ && !length)))
> > > @@ -841,10 +840,10 @@ static int mtk_dp_aux_do_transfer(struct
> > > mtk_dp *mtk_dp, bool is_read, u8 cmd,
> > > /* Wait for feedback from sink device. */
> > > ret = mtk_dp_aux_wait_for_completion(mtk_dp, is_read);
> > >
> > > - reply_cmd = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3624) &
> > > - AUX_RX_REPLY_COMMAND_AUX_TX_P0_MASK;
> > > + *reply_cmd = mtk_dp_read(mtk_dp, MTK_DP_AUX_P0_3624) &
> > > + AUX_RX_REPLY_COMMAND_AUX_TX_P0_MASK;
> > >
> > > - if (ret || reply_cmd) {
> > > + if (ret) {
> > > u32 phy_status = mtk_dp_read(mtk_dp,
> > > MTK_DP_AUX_P0_3628) &
> > > AUX_RX_PHY_STATE_AUX_TX_P0_MASK;
> > > if (phy_status !=
> > > AUX_RX_PHY_STATE_AUX_TX_P0_RX_IDLE) {
> > > @@ -2070,7 +2069,7 @@ static ssize_t mtk_dp_aux_transfer(struct
> > > drm_dp_aux *mtk_aux,
> > > ret = mtk_dp_aux_do_transfer(mtk_dp, is_read,
> > > request,
> > > msg->address +
> > > accessed_bytes,
> > > msg->buffer +
> > > accessed_bytes,
> > > - to_access);
> > > + to_access, &msg-
> > > >reply);
> > >
> > > if (ret) {
> > > drm_info(mtk_dp->drm_dev,
> > > @@ -2080,7 +2079,6 @@ static ssize_t mtk_dp_aux_transfer(struct
> > > drm_dp_aux *mtk_aux,
> > > accessed_bytes += to_access;
> > > } while (accessed_bytes < msg->size);
> > >
> > > - msg->reply = DP_AUX_NATIVE_REPLY_ACK |
> > > DP_AUX_I2C_REPLY_ACK;
> >
> > In your description, you just mention the retry count is 7 times, but
> > you does not mention you should change the reply. Why you modify
> > this?
> > And where is the 7 times retry?
> >
> > Regards,
> > Chun-Kuang.
> >
> > > return msg->size;
> > > err:
> > > msg->reply = DP_AUX_NATIVE_REPLY_NACK |
> > > DP_AUX_I2C_REPLY_NACK;
> > > --
> > > 2.18.0
> > >
>
> Hi CK:
>
> Thanks for your review!
>
> This patch is to fix some DP sinks that return AUX_DEFER, and the dp
> driver does not handle it according to the specification. DP_v1.4a
> spec 2.8.1.2 describes that if the sink returns AUX_DEFER, DPTX may
> retry later:
>
> The logic before the modification is that reply_cmd returns ETIMEDOUT
> if it is not AUX_ACK after the read operation, without considering the
> retry operation when returning AUX_DEFER;
>
> The modified logic is to add parameters to mtk_dp_aux_do_transfer() to
> store the return value of the sink. In the dmr_dp_helper.c file,
> drm_dp_i2c_do_msg calls aux->transfer and then performs retry
> operation according to msg->reply. The 7 times specified in the spec
> are also in this function defined in (max_retries).
Applied to mediatek-drm-next [1], thanks.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next
Regards,
Chun-Kuang.
>
> Best Regards!
> xinlei