Message ID | 20241111034126.2028401-2-airlied@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/2] nouveau: handle EBUSY and EAGAIN for GSP aux errors. | expand |
Reviewed-by: Lyude Paul <lyude@redhat.com> On Mon, 2024-11-11 at 13:41 +1000, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > eb284f4b3781 drm/nouveau/dp: Honor GSP link training retry timeouts > > tried to fix a problem with panel retires, however it appears > the auxch also needs the same treatment, so add the same retry > wrapper around it. > > This fixes some eDP panels after a suspend/resume cycle. > > Fixes: eb284f4b3781 ("drm/nouveau/dp: Honor GSP link training retry timeouts") > Signed-off-by: Dave Airlie <airlied@redhat.com> > --- > .../gpu/drm/nouveau/nvkm/engine/disp/r535.c | 57 +++++++++++-------- > 1 file changed, 34 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c > index 8f9aa3463c3c..99110ab2f44d 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c > @@ -1060,33 +1060,44 @@ r535_dp_aux_xfer(struct nvkm_outp *outp, u8 type, u32 addr, u8 *data, u8 *psize) > NV0073_CTRL_DP_AUXCH_CTRL_PARAMS *ctrl; > u8 size = *psize; > int ret; > + int retries; > > - ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom, NV0073_CTRL_CMD_DP_AUXCH_CTRL, sizeof(*ctrl)); > - if (IS_ERR(ctrl)) > - return PTR_ERR(ctrl); > + for (retries = 0; retries < 3; ++retries) { > + ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom, NV0073_CTRL_CMD_DP_AUXCH_CTRL, sizeof(*ctrl)); > + if (IS_ERR(ctrl)) > + return PTR_ERR(ctrl); > > - ctrl->subDeviceInstance = 0; > - ctrl->displayId = BIT(outp->index); > - ctrl->bAddrOnly = !size; > - ctrl->cmd = type; > - if (ctrl->bAddrOnly) { > - ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, REQ_TYPE, WRITE); > - ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, I2C_MOT, FALSE); > - } > - ctrl->addr = addr; > - ctrl->size = !ctrl->bAddrOnly ? (size - 1) : 0; > - memcpy(ctrl->data, data, size); > + ctrl->subDeviceInstance = 0; > + ctrl->displayId = BIT(outp->index); > + ctrl->bAddrOnly = !size; > + ctrl->cmd = type; > + if (ctrl->bAddrOnly) { > + ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, REQ_TYPE, WRITE); > + ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, I2C_MOT, FALSE); > + } > + ctrl->addr = addr; > + ctrl->size = !ctrl->bAddrOnly ? (size - 1) : 0; > + memcpy(ctrl->data, data, size); > > - ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl)); > - if (ret) { > - nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); > - return ret; > + ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl)); > + if ((ret == -EAGAIN || ret == -EBUSY) && ctrl->retryTimeMs) { > + /* > + * Device (likely an eDP panel) isn't ready yet, wait for the time specified > + * by GSP before retrying again > + */ > + nvkm_debug(&disp->engine.subdev, > + "Waiting %dms for GSP LT panel delay before retrying in AUX\n", > + ctrl->retryTimeMs); > + msleep(ctrl->retryTimeMs); > + nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); > + } else { > + memcpy(data, ctrl->data, size); > + *psize = ctrl->size; > + ret = ctrl->replyType; > + nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); > + break; > + } > } > - > - memcpy(data, ctrl->data, size); > - *psize = ctrl->size; > - ret = ctrl->replyType; > - nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); > return ret; > } >
On 11/11/24 13:41, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > eb284f4b3781 drm/nouveau/dp: Honor GSP link training retry timeouts > > tried to fix a problem with panel retires, however it appears > the auxch also needs the same treatment, so add the same retry > wrapper around it. > > This fixes some eDP panels after a suspend/resume cycle. This works fine on my laptop (though it doesn't fix the issues I see there unfortunately). The handling of -EAGAIN can be removed here too, but aside from that: Reviewed-by: Ben Skeggs <bskeggs@nvidia.com> > > Fixes: eb284f4b3781 ("drm/nouveau/dp: Honor GSP link training retry timeouts") > Signed-off-by: Dave Airlie <airlied@redhat.com> > --- > .../gpu/drm/nouveau/nvkm/engine/disp/r535.c | 57 +++++++++++-------- > 1 file changed, 34 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c > index 8f9aa3463c3c..99110ab2f44d 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c > @@ -1060,33 +1060,44 @@ r535_dp_aux_xfer(struct nvkm_outp *outp, u8 type, u32 addr, u8 *data, u8 *psize) > NV0073_CTRL_DP_AUXCH_CTRL_PARAMS *ctrl; > u8 size = *psize; > int ret; > + int retries; > > - ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom, NV0073_CTRL_CMD_DP_AUXCH_CTRL, sizeof(*ctrl)); > - if (IS_ERR(ctrl)) > - return PTR_ERR(ctrl); > + for (retries = 0; retries < 3; ++retries) { > + ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom, NV0073_CTRL_CMD_DP_AUXCH_CTRL, sizeof(*ctrl)); > + if (IS_ERR(ctrl)) > + return PTR_ERR(ctrl); > > - ctrl->subDeviceInstance = 0; > - ctrl->displayId = BIT(outp->index); > - ctrl->bAddrOnly = !size; > - ctrl->cmd = type; > - if (ctrl->bAddrOnly) { > - ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, REQ_TYPE, WRITE); > - ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, I2C_MOT, FALSE); > - } > - ctrl->addr = addr; > - ctrl->size = !ctrl->bAddrOnly ? (size - 1) : 0; > - memcpy(ctrl->data, data, size); > + ctrl->subDeviceInstance = 0; > + ctrl->displayId = BIT(outp->index); > + ctrl->bAddrOnly = !size; > + ctrl->cmd = type; > + if (ctrl->bAddrOnly) { > + ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, REQ_TYPE, WRITE); > + ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, I2C_MOT, FALSE); > + } > + ctrl->addr = addr; > + ctrl->size = !ctrl->bAddrOnly ? (size - 1) : 0; > + memcpy(ctrl->data, data, size); > > - ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl)); > - if (ret) { > - nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); > - return ret; > + ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl)); > + if ((ret == -EAGAIN || ret == -EBUSY) && ctrl->retryTimeMs) { > + /* > + * Device (likely an eDP panel) isn't ready yet, wait for the time specified > + * by GSP before retrying again > + */ > + nvkm_debug(&disp->engine.subdev, > + "Waiting %dms for GSP LT panel delay before retrying in AUX\n", > + ctrl->retryTimeMs); > + msleep(ctrl->retryTimeMs); > + nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); > + } else { > + memcpy(data, ctrl->data, size); > + *psize = ctrl->size; > + ret = ctrl->replyType; > + nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); > + break; > + } > } > - > - memcpy(data, ctrl->data, size); > - *psize = ctrl->size; > - ret = ctrl->replyType; > - nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); > return ret; > } >
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c index 8f9aa3463c3c..99110ab2f44d 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c @@ -1060,33 +1060,44 @@ r535_dp_aux_xfer(struct nvkm_outp *outp, u8 type, u32 addr, u8 *data, u8 *psize) NV0073_CTRL_DP_AUXCH_CTRL_PARAMS *ctrl; u8 size = *psize; int ret; + int retries; - ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom, NV0073_CTRL_CMD_DP_AUXCH_CTRL, sizeof(*ctrl)); - if (IS_ERR(ctrl)) - return PTR_ERR(ctrl); + for (retries = 0; retries < 3; ++retries) { + ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom, NV0073_CTRL_CMD_DP_AUXCH_CTRL, sizeof(*ctrl)); + if (IS_ERR(ctrl)) + return PTR_ERR(ctrl); - ctrl->subDeviceInstance = 0; - ctrl->displayId = BIT(outp->index); - ctrl->bAddrOnly = !size; - ctrl->cmd = type; - if (ctrl->bAddrOnly) { - ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, REQ_TYPE, WRITE); - ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, I2C_MOT, FALSE); - } - ctrl->addr = addr; - ctrl->size = !ctrl->bAddrOnly ? (size - 1) : 0; - memcpy(ctrl->data, data, size); + ctrl->subDeviceInstance = 0; + ctrl->displayId = BIT(outp->index); + ctrl->bAddrOnly = !size; + ctrl->cmd = type; + if (ctrl->bAddrOnly) { + ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, REQ_TYPE, WRITE); + ctrl->cmd = NVDEF_SET(ctrl->cmd, NV0073_CTRL, DP_AUXCH_CMD, I2C_MOT, FALSE); + } + ctrl->addr = addr; + ctrl->size = !ctrl->bAddrOnly ? (size - 1) : 0; + memcpy(ctrl->data, data, size); - ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl)); - if (ret) { - nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); - return ret; + ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl)); + if ((ret == -EAGAIN || ret == -EBUSY) && ctrl->retryTimeMs) { + /* + * Device (likely an eDP panel) isn't ready yet, wait for the time specified + * by GSP before retrying again + */ + nvkm_debug(&disp->engine.subdev, + "Waiting %dms for GSP LT panel delay before retrying in AUX\n", + ctrl->retryTimeMs); + msleep(ctrl->retryTimeMs); + nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); + } else { + memcpy(data, ctrl->data, size); + *psize = ctrl->size; + ret = ctrl->replyType; + nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); + break; + } } - - memcpy(data, ctrl->data, size); - *psize = ctrl->size; - ret = ctrl->replyType; - nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); return ret; }