Skip to content

Commit 4ae3a20

Browse files
airliedairlied
committed
nouveau/gsp: don't free ctrl messages on errors
It looks like for some messages the upper layers need to get access to the results of the message so we can interpret it. Rework the ctrl push interface to not free things and cleanup properly whereever it errors out. Requested-by: Lyude Signed-off-by: Dave Airlie <airlied@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20231222043308.3090089-9-airlied@gmail.com
1 parent 59f6a3d commit 4ae3a20

3 files changed

Lines changed: 100 additions & 61 deletions

File tree

‎drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ struct nvkm_gsp {
187187
void (*rpc_done)(struct nvkm_gsp *gsp, void *repv);
188188

189189
void *(*rm_ctrl_get)(struct nvkm_gsp_object *, u32 cmd, u32 argc);
190-
void *(*rm_ctrl_push)(struct nvkm_gsp_object *, void *argv, u32 repc);
190+
int (*rm_ctrl_push)(struct nvkm_gsp_object *, void **argv, u32 repc);
191191
void (*rm_ctrl_done)(struct nvkm_gsp_object *, void *repv);
192192

193193
void *(*rm_alloc_get)(struct nvkm_gsp_object *, u32 oclass, u32 argc);
@@ -265,7 +265,7 @@ nvkm_gsp_rm_ctrl_get(struct nvkm_gsp_object *object, u32 cmd, u32 argc)
265265
return object->client->gsp->rm->rm_ctrl_get(object, cmd, argc);
266266
}
267267

268-
static inline void *
268+
static inline int
269269
nvkm_gsp_rm_ctrl_push(struct nvkm_gsp_object *object, void *argv, u32 repc)
270270
{
271271
return object->client->gsp->rm->rm_ctrl_push(object, argv, repc);
@@ -275,21 +275,24 @@ static inline void *
275275
nvkm_gsp_rm_ctrl_rd(struct nvkm_gsp_object *object, u32 cmd, u32 repc)
276276
{
277277
void *argv = nvkm_gsp_rm_ctrl_get(object, cmd, repc);
278+
int ret;
278279

279280
if (IS_ERR(argv))
280281
return argv;
281282

282-
return nvkm_gsp_rm_ctrl_push(object, argv, repc);
283+
ret = nvkm_gsp_rm_ctrl_push(object, &argv, repc);
284+
if (ret)
285+
return ERR_PTR(ret);
286+
return argv;
283287
}
284288

285289
static inline int
286290
nvkm_gsp_rm_ctrl_wr(struct nvkm_gsp_object *object, void *argv)
287291
{
288-
void *repv = nvkm_gsp_rm_ctrl_push(object, argv, 0);
289-
290-
if (IS_ERR(repv))
291-
return PTR_ERR(repv);
292+
int ret = nvkm_gsp_rm_ctrl_push(object, &argv, 0);
292293

294+
if (ret)
295+
return ret;
293296
return 0;
294297
}
295298

‎drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c‎

Lines changed: 69 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ r535_sor_bl_get(struct nvkm_ior *sor)
282282
{
283283
struct nvkm_disp *disp = sor->disp;
284284
NV0073_CTRL_SPECIFIC_BACKLIGHT_BRIGHTNESS_PARAMS *ctrl;
285-
int lvl;
285+
int ret, lvl;
286286

287287
ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
288288
NV0073_CTRL_CMD_SPECIFIC_GET_BACKLIGHT_BRIGHTNESS,
@@ -292,9 +292,11 @@ r535_sor_bl_get(struct nvkm_ior *sor)
292292

293293
ctrl->displayId = BIT(sor->asy.outp->index);
294294

295-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
296-
if (IS_ERR(ctrl))
297-
return PTR_ERR(ctrl);
295+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
296+
if (ret) {
297+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
298+
return ret;
299+
}
298300

299301
lvl = ctrl->brightness;
300302
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
@@ -649,9 +651,11 @@ r535_conn_new(struct nvkm_disp *disp, u32 id)
649651
ctrl->subDeviceInstance = 0;
650652
ctrl->displayId = BIT(id);
651653

652-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
653-
if (IS_ERR(ctrl))
654-
return (void *)ctrl;
654+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
655+
if (ret) {
656+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
657+
return ERR_PTR(ret);
658+
}
655659

656660
list_for_each_entry(conn, &disp->conns, head) {
657661
if (conn->index == ctrl->data[0].index) {
@@ -686,7 +690,7 @@ r535_outp_acquire(struct nvkm_outp *outp, bool hda)
686690
struct nvkm_disp *disp = outp->disp;
687691
struct nvkm_ior *ior;
688692
NV0073_CTRL_DFP_ASSIGN_SOR_PARAMS *ctrl;
689-
int or;
693+
int ret, or;
690694

691695
ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
692696
NV0073_CTRL_CMD_DFP_ASSIGN_SOR, sizeof(*ctrl));
@@ -699,9 +703,11 @@ r535_outp_acquire(struct nvkm_outp *outp, bool hda)
699703
if (hda)
700704
ctrl->flags |= NVDEF(NV0073_CTRL, DFP_ASSIGN_SOR_FLAGS, AUDIO, OPTIMAL);
701705

702-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
703-
if (IS_ERR(ctrl))
704-
return PTR_ERR(ctrl);
706+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
707+
if (ret) {
708+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
709+
return ret;
710+
}
705711

706712
for (or = 0; or < ARRAY_SIZE(ctrl->sorAssignListWithTag); or++) {
707713
if (ctrl->sorAssignListWithTag[or].displayMask & BIT(outp->index)) {
@@ -727,6 +733,7 @@ static int
727733
r535_disp_head_displayid(struct nvkm_disp *disp, int head, u32 *displayid)
728734
{
729735
NV0073_CTRL_SYSTEM_GET_ACTIVE_PARAMS *ctrl;
736+
int ret;
730737

731738
ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
732739
NV0073_CTRL_CMD_SYSTEM_GET_ACTIVE, sizeof(*ctrl));
@@ -736,9 +743,11 @@ r535_disp_head_displayid(struct nvkm_disp *disp, int head, u32 *displayid)
736743
ctrl->subDeviceInstance = 0;
737744
ctrl->head = head;
738745

739-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
740-
if (IS_ERR(ctrl))
741-
return PTR_ERR(ctrl);
746+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
747+
if (ret) {
748+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
749+
return ret;
750+
}
742751

743752
*displayid = ctrl->displayId;
744753
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
@@ -772,9 +781,11 @@ r535_outp_inherit(struct nvkm_outp *outp)
772781
ctrl->subDeviceInstance = 0;
773782
ctrl->displayId = displayid;
774783

775-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
776-
if (IS_ERR(ctrl))
784+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
785+
if (ret) {
786+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
777787
return NULL;
788+
}
778789

779790
id = ctrl->index;
780791
proto = ctrl->protocol;
@@ -825,16 +836,19 @@ r535_outp_dfp_get_info(struct nvkm_outp *outp)
825836
{
826837
NV0073_CTRL_DFP_GET_INFO_PARAMS *ctrl;
827838
struct nvkm_disp *disp = outp->disp;
839+
int ret;
828840

829841
ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom, NV0073_CTRL_CMD_DFP_GET_INFO, sizeof(*ctrl));
830842
if (IS_ERR(ctrl))
831843
return PTR_ERR(ctrl);
832844

833845
ctrl->displayId = BIT(outp->index);
834846

835-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
836-
if (IS_ERR(ctrl))
837-
return PTR_ERR(ctrl);
847+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
848+
if (ret) {
849+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
850+
return ret;
851+
}
838852

839853
nvkm_debug(&disp->engine.subdev, "DFP %08x: flags:%08x flags2:%08x\n",
840854
ctrl->displayId, ctrl->flags, ctrl->flags2);
@@ -858,9 +872,11 @@ r535_outp_detect(struct nvkm_outp *outp)
858872
ctrl->subDeviceInstance = 0;
859873
ctrl->displayMask = BIT(outp->index);
860874

861-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
862-
if (IS_ERR(ctrl))
863-
return PTR_ERR(ctrl);
875+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
876+
if (ret) {
877+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
878+
return ret;
879+
}
864880

865881
if (ctrl->displayMask & BIT(outp->index)) {
866882
ret = r535_outp_dfp_get_info(outp);
@@ -895,6 +911,7 @@ r535_dp_mst_id_get(struct nvkm_outp *outp, u32 *pid)
895911
{
896912
NV0073_CTRL_CMD_DP_TOPOLOGY_ALLOCATE_DISPLAYID_PARAMS *ctrl;
897913
struct nvkm_disp *disp = outp->disp;
914+
int ret;
898915

899916
ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
900917
NV0073_CTRL_CMD_DP_TOPOLOGY_ALLOCATE_DISPLAYID,
@@ -904,9 +921,11 @@ r535_dp_mst_id_get(struct nvkm_outp *outp, u32 *pid)
904921

905922
ctrl->subDeviceInstance = 0;
906923
ctrl->displayId = BIT(outp->index);
907-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
908-
if (IS_ERR(ctrl))
909-
return PTR_ERR(ctrl);
924+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
925+
if (ret) {
926+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
927+
return ret;
928+
}
910929

911930
*pid = ctrl->displayIdAssigned;
912931
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
@@ -964,9 +983,11 @@ r535_dp_train_target(struct nvkm_outp *outp, u8 target, bool mst, u8 link_nr, u8
964983
!(outp->dp.dpcd[DPCD_RC03] & DPCD_RC03_TPS4_SUPPORTED))
965984
ctrl->cmd |= NVDEF(NV0073_CTRL, DP_CMD, POST_LT_ADJ_REQ_GRANTED, YES);
966985

967-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
968-
if (IS_ERR(ctrl))
969-
return PTR_ERR(ctrl);
986+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
987+
if (ret) {
988+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
989+
return ret;
990+
}
970991

971992
ret = ctrl->err ? -EIO : 0;
972993
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
@@ -1036,9 +1057,11 @@ r535_dp_aux_xfer(struct nvkm_outp *outp, u8 type, u32 addr, u8 *data, u8 *psize)
10361057
ctrl->size = !ctrl->bAddrOnly ? (size - 1) : 0;
10371058
memcpy(ctrl->data, data, size);
10381059

1039-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
1040-
if (IS_ERR(ctrl))
1060+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
1061+
if (ret) {
1062+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
10411063
return PTR_ERR(ctrl);
1064+
}
10421065

10431066
memcpy(data, ctrl->data, size);
10441067
*psize = ctrl->size;
@@ -1111,10 +1134,13 @@ r535_tmds_edid_get(struct nvkm_outp *outp, u8 *data, u16 *psize)
11111134
ctrl->subDeviceInstance = 0;
11121135
ctrl->displayId = BIT(outp->index);
11131136

1114-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
1115-
if (IS_ERR(ctrl))
1116-
return PTR_ERR(ctrl);
1137+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
1138+
if (ret) {
1139+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
1140+
return ret;
1141+
}
11171142

1143+
ret = -E2BIG;
11181144
if (ctrl->bufferSize <= *psize) {
11191145
memcpy(data, ctrl->edidBuffer, ctrl->bufferSize);
11201146
*psize = ctrl->bufferSize;
@@ -1153,9 +1179,11 @@ r535_outp_new(struct nvkm_disp *disp, u32 id)
11531179
ctrl->subDeviceInstance = 0;
11541180
ctrl->displayId = BIT(id);
11551181

1156-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
1157-
if (IS_ERR(ctrl))
1158-
return PTR_ERR(ctrl);
1182+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
1183+
if (ret) {
1184+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
1185+
return ret;
1186+
}
11591187

11601188
switch (ctrl->type) {
11611189
case NV0073_CTRL_SPECIFIC_OR_TYPE_NONE:
@@ -1229,9 +1257,11 @@ r535_outp_new(struct nvkm_disp *disp, u32 id)
12291257

12301258
ctrl->sorIndex = ~0;
12311259

1232-
ctrl = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, ctrl, sizeof(*ctrl));
1233-
if (IS_ERR(ctrl))
1234-
return PTR_ERR(ctrl);
1260+
ret = nvkm_gsp_rm_ctrl_push(&disp->rm.objcom, &ctrl, sizeof(*ctrl));
1261+
if (ret) {
1262+
nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
1263+
return ret;
1264+
}
12351265

12361266
switch (NVVAL_GET(ctrl->maxLinkRate, NV0073_CTRL_CMD, DP_GET_CAPS, MAX_LINK_RATE)) {
12371267
case NV0073_CTRL_CMD_DP_GET_CAPS_MAX_LINK_RATE_1_62:

‎drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -599,13 +599,13 @@ r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *argv, u32 repc)
599599

600600
if (rpc->status) {
601601
ret = ERR_PTR(r535_rpc_status_to_errno(rpc->status));
602-
if (ret != -EAGAIN)
602+
if (PTR_ERR(ret) != -EAGAIN)
603603
nvkm_error(&gsp->subdev, "RM_ALLOC: 0x%x\n", rpc->status);
604604
} else {
605605
ret = repc ? rpc->params : NULL;
606606
}
607607

608-
if (IS_ERR_OR_NULL(ret))
608+
if (ret)
609609
nvkm_gsp_rpc_done(gsp, rpc);
610610

611611
return ret;
@@ -639,30 +639,34 @@ r535_gsp_rpc_rm_ctrl_done(struct nvkm_gsp_object *object, void *repv)
639639
{
640640
rpc_gsp_rm_control_v03_00 *rpc = container_of(repv, typeof(*rpc), params);
641641

642+
if (!repv)
643+
return;
642644
nvkm_gsp_rpc_done(object->client->gsp, rpc);
643645
}
644646

645-
static void *
646-
r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void *argv, u32 repc)
647+
static int
648+
r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **argv, u32 repc)
647649
{
648-
rpc_gsp_rm_control_v03_00 *rpc = container_of(argv, typeof(*rpc), params);
650+
rpc_gsp_rm_control_v03_00 *rpc = container_of((*argv), typeof(*rpc), params);
649651
struct nvkm_gsp *gsp = object->client->gsp;
650-
void *ret;
652+
int ret = 0;
651653

652654
rpc = nvkm_gsp_rpc_push(gsp, rpc, true, repc);
653-
if (IS_ERR_OR_NULL(rpc))
654-
return rpc;
655+
if (IS_ERR_OR_NULL(rpc)) {
656+
*argv = NULL;
657+
return PTR_ERR(rpc);
658+
}
655659

656660
if (rpc->status) {
657-
ret = ERR_PTR(r535_rpc_status_to_errno(rpc->status));
661+
ret = r535_rpc_status_to_errno(rpc->status);
658662
if (ret != -EAGAIN)
659663
nvkm_error(&gsp->subdev, "cli:0x%08x obj:0x%08x ctrl cmd:0x%08x failed: 0x%08x\n",
660664
object->client->object.handle, object->handle, rpc->cmd, rpc->status);
661-
} else {
662-
ret = repc ? rpc->params : NULL;
663665
}
664666

665-
if (IS_ERR_OR_NULL(ret))
667+
if (repc)
668+
*argv = rpc->params;
669+
else
666670
nvkm_gsp_rpc_done(gsp, rpc);
667671

668672
return ret;
@@ -860,9 +864,11 @@ r535_gsp_intr_get_table(struct nvkm_gsp *gsp)
860864
if (IS_ERR(ctrl))
861865
return PTR_ERR(ctrl);
862866

863-
ctrl = nvkm_gsp_rm_ctrl_push(&gsp->internal.device.subdevice, ctrl, sizeof(*ctrl));
864-
if (WARN_ON(IS_ERR(ctrl)))
865-
return PTR_ERR(ctrl);
867+
ret = nvkm_gsp_rm_ctrl_push(&gsp->internal.device.subdevice, &ctrl, sizeof(*ctrl));
868+
if (WARN_ON(ret)) {
869+
nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, ctrl);
870+
return ret;
871+
}
866872

867873
for (unsigned i = 0; i < ctrl->tableLen; i++) {
868874
enum nvkm_subdev_type type;

0 commit comments

Comments
 (0)