From 704036c2a3e84a1d1a42d9511e3527e029c37e59 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 12 Oct 2020 16:44:43 +0900 Subject: [PATCH 01/16] drm/vc4: crtc: Keep the previously assigned HVS FIFO The HVS FIFOs are currently assigned each time we have an atomic_check for all the enabled CRTCs. However, if we are running multiple outputs in parallel and we happen to disable the first (by index) CRTC, we end up changing the assigned FIFO of the second CRTC without disabling and reenabling the pixelvalve which ends up in a stall and eventually a VBLANK timeout. In order to fix this, we can create a special value for our assigned channel to mark it as disabled, and if our CRTC already had an assigned channel in its previous state, we keep on using it. Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically") Signed-off-by: Maxime Ripard [hoegeun.kwon: Needed to fix page flip issue of dual hdmi.] Signed-off-by: Hoegeun Kwon Change-Id: I70e0d6b563d5c7f22a1a7e1527a401ac42395b78 --- drivers/gpu/drm/vc4/vc4_crtc.c | 12 +++++++++--- drivers/gpu/drm/vc4/vc4_drv.h | 1 + drivers/gpu/drm/vc4/vc4_kms.c | 21 +++++++++++++++------ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index f8fa076..2eabf02f 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -1088,12 +1088,18 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc, static void vc4_crtc_reset(struct drm_crtc *crtc) { + struct vc4_crtc_state *vc4_crtc_state; + if (crtc->state) vc4_crtc_destroy_state(crtc, crtc->state); - crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL); - if (crtc->state) - crtc->state->crtc = crtc; + vc4_crtc_state = kzalloc(sizeof(*vc4_crtc_state), GFP_KERNEL); + if (!vc4_crtc_state) + return; + + vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED; + crtc->state = &vc4_crtc_state->base; + crtc->state->crtc = crtc; } static const struct drm_crtc_funcs vc4_crtc_funcs = { diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 9273595..457fd68 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -511,6 +511,7 @@ struct vc4_crtc_state { unsigned int bottom; } margins; }; +#define VC4_HVS_CHANNEL_DISABLED ((unsigned int) -1) static inline struct vc4_crtc_state * to_vc4_crtc_state(struct drm_crtc_state *crtc_state) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 96aadcb..77745ad 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -585,7 +585,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) { unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0); struct vc4_dev *vc4 = to_vc4_dev(state->dev); - struct drm_crtc_state *crtc_state; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_crtc *crtc; int i, ret; @@ -598,6 +598,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) * modified. */ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { + struct drm_crtc_state *crtc_state; if (!crtc->state->enable) continue; @@ -606,15 +607,23 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) return PTR_ERR(crtc_state); } - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { - struct vc4_crtc_state *vc4_crtc_state = - to_vc4_crtc_state(crtc_state); + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { + struct vc4_crtc_state *new_vc4_crtc_state = + to_vc4_crtc_state(new_crtc_state); struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); bool is_assigned = false; unsigned int channel; - if (!crtc_state->enable || vc4->firmware_kms) + if (old_crtc_state->enable && !new_crtc_state->enable) + new_vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED; + + if (!new_crtc_state->enable) + continue; + + if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED) { + unassigned_channels &= ~BIT(new_vc4_crtc_state->assigned_channel); continue; + } /* * The problem we have to solve here is that we have @@ -646,7 +655,7 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) if (!(BIT(channel) & vc4_crtc->data->hvs_available_channels)) continue; - vc4_crtc_state->assigned_channel = channel; + new_vc4_crtc_state->assigned_channel = channel; unassigned_channels &= ~BIT(channel); is_assigned = true; break; -- 2.7.4 From b6cf6945c8bb5da757776af2e4df8cebf6383368 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 12 Oct 2020 16:55:43 +0900 Subject: [PATCH 02/16] drm/vc4: kms: Split the HVS muxing check in a separate function The code that assigns HVS channels during atomic_check is starting to grow a bit big, let's move it into a separate function. Signed-off-by: Maxime Ripard [hoegeun.kwon: Needed to fix page flip issue of dual hdmi.] Signed-off-by: Hoegeun Kwon Change-Id: Icf636a846282981d21ad3e6652a3ec51f14993a4 --- drivers/gpu/drm/vc4/vc4_kms.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 77745ad..9a56596 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -580,14 +580,14 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = { #define NUM_OUTPUTS 6 #define NUM_CHANNELS 3 -static int -vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) +static int vc4_pv_muxing_atomic_check(struct drm_device *dev, + struct drm_atomic_state *state) { unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0); struct vc4_dev *vc4 = to_vc4_dev(state->dev); struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_crtc *crtc; - int i, ret; + unsigned int i; /* * Since the HVS FIFOs are shared across all the pixelvalves and @@ -665,6 +665,18 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) return -EINVAL; } + return 0; +} + +static int +vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) +{ + int ret; + + ret = vc4_pv_muxing_atomic_check(dev, state); + if (ret) + return ret; + ret = vc4_ctm_atomic_check(dev, state); if (ret < 0) return ret; -- 2.7.4 From 47951c47a46ca69c88001856f0e4236b2428fd32 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 8 Oct 2020 13:25:17 +0200 Subject: [PATCH 03/16] drm/vc4: kms: Document the muxing corner cases We've had a number of muxing corner-cases with specific ways to reproduce them, so let's document them to make sure they aren't lost and introduce regressions later on. Signed-off-by: Maxime Ripard [hoegeun.kwon: Needed to fix page flip issue of dual hdmi.] Signed-off-by: Hoegeun Kwon Change-Id: Icc4f82cd0c22f7b0b7e3c33be37d2e9d287638e4 --- drivers/gpu/drm/vc4/vc4_kms.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 9a56596..5beb875 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -580,6 +580,28 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = { #define NUM_OUTPUTS 6 #define NUM_CHANNELS 3 +/* + * The BCM2711 HVS has up to 7 output connected to the pixelvalves and + * the TXP (and therefore all the CRTCs found on that platform). + * + * The naive (and our initial) implementation would just iterate over + * all the active CRTCs, try to find a suitable FIFO, and then remove it + * from the available FIFOs pool. However, there's a few corner cases + * that need to be considered: + * + * - When running in a dual-display setup (so with two CRTCs involved), + * we can update the state of a single CRTC (for example by changing + * its mode using xrandr under X11) without affecting the other. In + * this case, the other CRTC wouldn't be in the state at all, so we + * need to consider all the running CRTCs in the DRM device to assign + * a FIFO, not just the one in the state. + * + * - Since we need the pixelvalve to be disabled and enabled back when + * the FIFO is changed, we should keep the FIFO assigned for as long + * as the CRTC is enabled, only considering it free again once that + * CRTC has been disabled. This can be tested by booting X11 on a + * single display, and changing the resolution down and then back up. + */ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) { -- 2.7.4 From f1b82fc8050c75fd0e292e492e51d103f5847c26 Mon Sep 17 00:00:00 2001 From: Hoegeun Kwon Date: Mon, 12 Oct 2020 21:10:58 +0900 Subject: [PATCH 04/16] drm/vc4: kms: Fix to split vc4 and vc5 hvs pv muxing commit In order to use dual HDMI normally, vc4 and vc5 should be separated. Split vc4 hvs pv muxing commit and vc5 referring to patch[1]. [1] drm/vc4: crtc: Assign output to channel automatically https://cgit.freedesktop.org/drm/drm-misc/commit/?id=87ebcd42fb7b8d1d3269007a621e41ae96a0077e Change-Id: I5427cfd885c4643f764f8f84bed0f017c5d2a562 Signed-off-by: Hoegeun Kwon --- drivers/gpu/drm/vc4/vc4_kms.c | 78 ++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 5beb875..367cead 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -157,6 +157,45 @@ static void vc4_hvs_pv_muxing_commit(struct vc4_dev *vc4, { struct drm_crtc_state *crtc_state; struct drm_crtc *crtc; + unsigned int i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state); + u32 dispctrl; + u32 dsp3_mux; + + if (!crtc_state->active) + continue; + + if (vc4_state->assigned_channel != 2) + continue; + + /* + * SCALER_DISPCTRL_DSP3 = X, where X < 2 means 'connect DSP3 to + * FIFO X'. + * SCALER_DISPCTRL_DSP3 = 3 means 'disable DSP 3'. + * + * DSP3 is connected to FIFO2 unless the transposer is + * enabled. In this case, FIFO 2 is directly accessed by the + * TXP IP, and we need to disable the FIFO2 -> pixelvalve1 + * route. + */ + if (vc4_state->feed_txp) + dsp3_mux = VC4_SET_FIELD(3, SCALER_DISPCTRL_DSP3_MUX); + else + dsp3_mux = VC4_SET_FIELD(2, SCALER_DISPCTRL_DSP3_MUX); + + dispctrl = HVS_READ(SCALER_DISPCTRL) & + ~SCALER_DISPCTRL_DSP3_MUX_MASK; + HVS_WRITE(SCALER_DISPCTRL, dispctrl | dsp3_mux); + } +} + +static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4, + struct drm_atomic_state *state) +{ + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; unsigned char dsp2_mux = 0; unsigned char dsp3_mux = 3; unsigned char dsp4_mux = 3; @@ -173,7 +212,7 @@ static void vc4_hvs_pv_muxing_commit(struct vc4_dev *vc4, switch (vc4_crtc->data->hvs_output) { case 2: - dsp2_mux = (vc4_state->assigned_channel == 2) ? 1 : 0; + dsp2_mux = (vc4_state->assigned_channel == 2) ? 0 : 1; break; case 3: @@ -194,30 +233,27 @@ static void vc4_hvs_pv_muxing_commit(struct vc4_dev *vc4, } reg = HVS_READ(SCALER_DISPECTRL); - if (FIELD_GET(SCALER_DISPECTRL_DSP2_MUX_MASK, reg) != dsp2_mux) - HVS_WRITE(SCALER_DISPECTRL, - (reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) | - VC4_SET_FIELD(dsp2_mux, SCALER_DISPECTRL_DSP2_MUX)); + HVS_WRITE(SCALER_DISPECTRL, + (reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) | + VC4_SET_FIELD(dsp2_mux, SCALER_DISPECTRL_DSP2_MUX)); reg = HVS_READ(SCALER_DISPCTRL); - if (FIELD_GET(SCALER_DISPCTRL_DSP3_MUX_MASK, reg) != dsp3_mux) - HVS_WRITE(SCALER_DISPCTRL, - (reg & ~SCALER_DISPCTRL_DSP3_MUX_MASK) | - VC4_SET_FIELD(dsp3_mux, SCALER_DISPCTRL_DSP3_MUX)); + HVS_WRITE(SCALER_DISPCTRL, + (reg & ~SCALER_DISPCTRL_DSP3_MUX_MASK) | + VC4_SET_FIELD(dsp3_mux, SCALER_DISPCTRL_DSP3_MUX)); reg = HVS_READ(SCALER_DISPEOLN); - if (FIELD_GET(SCALER_DISPEOLN_DSP4_MUX_MASK, reg) != dsp4_mux) - HVS_WRITE(SCALER_DISPEOLN, - (reg & ~SCALER_DISPEOLN_DSP4_MUX_MASK) | - VC4_SET_FIELD(dsp4_mux, SCALER_DISPEOLN_DSP4_MUX)); + HVS_WRITE(SCALER_DISPEOLN, + (reg & ~SCALER_DISPEOLN_DSP4_MUX_MASK) | + VC4_SET_FIELD(dsp4_mux, SCALER_DISPEOLN_DSP4_MUX)); reg = HVS_READ(SCALER_DISPDITHER); - if (FIELD_GET(SCALER_DISPDITHER_DSP5_MUX_MASK, reg) != dsp5_mux) - HVS_WRITE(SCALER_DISPDITHER, - (reg & ~SCALER_DISPDITHER_DSP5_MUX_MASK) | - VC4_SET_FIELD(dsp5_mux, SCALER_DISPDITHER_DSP5_MUX)); + HVS_WRITE(SCALER_DISPDITHER, + (reg & ~SCALER_DISPDITHER_DSP5_MUX_MASK) | + VC4_SET_FIELD(dsp5_mux, SCALER_DISPDITHER_DSP5_MUX)); } + static void vc4_atomic_complete_commit(struct drm_atomic_state *state) { @@ -248,10 +284,12 @@ vc4_atomic_complete_commit(struct drm_atomic_state *state) drm_atomic_helper_commit_modeset_disables(dev, state); - if (!vc4->firmware_kms) { - vc4_ctm_commit(vc4, state); + vc4_ctm_commit(vc4, state); + + if (vc4->hvs->hvs5) + vc5_hvs_pv_muxing_commit(vc4, state); + else vc4_hvs_pv_muxing_commit(vc4, state); - } drm_atomic_helper_commit_planes(dev, state, 0); -- 2.7.4 From 5d2fec61a25bacc49ee8e84b3c19aee1522f7289 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 8 Oct 2020 13:25:18 +0200 Subject: [PATCH 05/16] drm/vc4: kms: Don't disable the muxing of an active CRTC The current HVS muxing code will consider the CRTCs in a given state to setup their muxing in the HVS, and disable the other CRTCs muxes. However, it's valid to only update a single CRTC with a state, and in this situation we would mux out a CRTC that was enabled but left untouched by the new state. Fix this by considering all the CRTCs in the state and the ones with a state and active. Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically") Signed-off-by: Maxime Ripard [hoegeun.kwon: Needed to fix page flip issue of dual hdmi.] Signed-off-by: Hoegeun Kwon Change-Id: Idb6e79214f52e470e9fcf5a0579873402c631eb2 --- drivers/gpu/drm/vc4/vc4_kms.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 367cead..06b291f 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -191,6 +191,23 @@ static void vc4_hvs_pv_muxing_commit(struct vc4_dev *vc4, } } +static struct drm_crtc_state * +drm_atomic_get_new_or_current_crtc_state(struct drm_atomic_state *state, + struct drm_crtc *crtc) +{ + struct drm_crtc_state *crtc_state; + + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + if (crtc_state) + return crtc_state; + + return crtc->state; +} + +#define for_each_new_or_current_crtc_state(__state, crtc, crtc_state) \ + list_for_each_entry(crtc, &__state->dev->mode_config.crtc_list, head) \ + for_each_if(crtc_state = drm_atomic_get_new_or_current_crtc_state(__state, crtc)) + static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4, struct drm_atomic_state *state) { @@ -200,16 +217,16 @@ static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4, unsigned char dsp3_mux = 3; unsigned char dsp4_mux = 3; unsigned char dsp5_mux = 3; - unsigned int i; u32 reg; - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { - struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state); + for_each_new_or_current_crtc_state(state, crtc, crtc_state) { struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); + struct vc4_crtc_state *vc4_state; if (!crtc_state->active) continue; + vc4_state = to_vc4_crtc_state(crtc_state); switch (vc4_crtc->data->hvs_output) { case 2: dsp2_mux = (vc4_state->assigned_channel == 2) ? 0 : 1; -- 2.7.4 From e805316d5d44b1f1f080fd8ae8a34b69329d940c Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 12 Oct 2020 12:44:25 +0900 Subject: [PATCH 06/16] drm/vc4: kms: Fix VBLANK reporting on a disabled CRTC If a CRTC is enabled but not active, and that we're then doing a page flip on another CRTC, drm_atomic_get_crtc_state will bring the first CRTC state into the global state, and will make us wait for its vblank as well, even though that might never occur. Fix this by considering all the enabled CRTCs by either using their new state in the global state, or using their current state if they aren't part of the new state being checked, to remove their assigned channel from the pool before started to assign channels to CRTCs enabled by the state. Fixes: 87ebcd42fb7b ("drm/vc4: crtc: Assign output to channel automatically") Signed-off-by: Maxime Ripard [hoegeun.kwon: Needed to fix page flip issue of dual hdmi.] Signed-off-by: Hoegeun Kwon Change-Id: Ic8b8083a652df142a17ec0d50f6c2572494ba3c0 --- drivers/gpu/drm/vc4/vc4_kms.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 06b291f..a552221 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -651,6 +651,14 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = { * need to consider all the running CRTCs in the DRM device to assign * a FIFO, not just the one in the state. * + * - To fix the above, we can't use drm_atomic_get_crtc_state on all + * enabled CRTCs to pull their CRTC state into the global state, since + * a page flip would start considering their vblank to complete. Since + * we don't have a guarantee that they are actually active, that + * vblank might never happen, and shouldn't even be considered if we + * want to do a page flip on a single CRTC. That can be tested by + * doing a modetest -v first on HDMI1 and then on HDMI0. + * * - Since we need the pixelvalve to be disabled and enabled back when * the FIFO is changed, we should keep the FIFO assigned for as long * as the CRTC is enabled, only considering it free again once that @@ -661,8 +669,8 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) { unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0); - struct vc4_dev *vc4 = to_vc4_dev(state->dev); struct drm_crtc_state *old_crtc_state, *new_crtc_state; + struct drm_crtc_state *crtc_state; struct drm_crtc *crtc; unsigned int i; @@ -674,14 +682,13 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, * the same CRTCs, instead of evaluating only the CRTC being * modified. */ - list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { - struct drm_crtc_state *crtc_state; - if (!crtc->state->enable) + for_each_new_or_current_crtc_state(state, crtc, crtc_state) { + struct vc4_crtc_state *vc4_crtc_state; + if (!crtc_state->enable) continue; - crtc_state = drm_atomic_get_crtc_state(state, crtc); - if (IS_ERR(crtc_state)) - return PTR_ERR(crtc_state); + vc4_crtc_state = to_vc4_crtc_state(crtc_state); + unassigned_channels &= ~BIT(vc4_crtc_state->assigned_channel); } for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { @@ -697,10 +704,8 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, if (!new_crtc_state->enable) continue; - if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED) { - unassigned_channels &= ~BIT(new_vc4_crtc_state->assigned_channel); + if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED) continue; - } /* * The problem we have to solve here is that we have -- 2.7.4 From f7cc59406d8bae7f86d817f3b126597bc809984a Mon Sep 17 00:00:00 2001 From: Seung-Woo Kim Date: Mon, 26 Oct 2020 16:37:19 +0900 Subject: [PATCH 07/16] staging: mmal-vchiq: Fix memory leak for vchi_instance The vchi_instance is allocated with vchiq_initialise() but never handled properly. Fix memory leak for the vchi_instance. This fixes below kmemleak report: [<000000002312557f>] kmem_cache_alloc_trace+0x1e0/0x348 [<000000002ee5d470>] vchiq_initialise+0xd0/0x258 [<000000009b51d8f4>] vchi_initialise+0x84/0xc8 [<00000000a58f68c5>] vchiq_mmal_init+0xb0/0x2f0 [<000000006c68d7bf>] bcm2835_mmal_probe+0x90/0x660 ... Change-Id: Ib10f194aa4058b3e39ec0a250fa36fd00daf9feb Reported-by: Jaehoon Chung Signed-off-by: Seung-Woo Kim --- drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c index 61a3593..486e1d2 100644 --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c @@ -191,6 +191,9 @@ struct vchiq_mmal_instance { /* ordered workqueue to process all bulk operations */ struct workqueue_struct *bulk_wq; + + /* Opaque handle for a VCHI instance */ + VCHI_INSTANCE_T vchi_instance; }; static struct mmal_msg_context * @@ -2086,6 +2089,7 @@ int vchiq_mmal_finalise(struct vchiq_mmal_instance *instance) mutex_unlock(&instance->vchiq_mutex); + vchi_disconnect(instance->vchi_instance); flush_workqueue(instance->bulk_wq); destroy_workqueue(instance->bulk_wq); @@ -2102,6 +2106,7 @@ EXPORT_SYMBOL_GPL(vchiq_mmal_finalise); int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance) { int status; + int err = -ENODEV; struct vchiq_mmal_instance *instance; static VCHI_INSTANCE_T vchi_instance; struct service_creation params = { @@ -2135,17 +2140,21 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance) status = vchi_connect(vchi_instance); if (status) { pr_err("Failed to connect VCHI instance (status=%d)\n", status); - return -EIO; + err = -EIO; + goto err_disconnect_vchi; } instance = kzalloc(sizeof(*instance), GFP_KERNEL); - if (!instance) - return -ENOMEM; + if (!instance) { + err = -ENOMEM; + goto err_disconnect_vchi; + } mutex_init(&instance->vchiq_mutex); instance->bulk_scratch = vmalloc(PAGE_SIZE); + instance->vchi_instance = vchi_instance; mutex_init(&instance->context_map_lock); idr_init_base(&instance->context_map, 1); @@ -2176,6 +2185,8 @@ err_close_services: err_free: vfree(instance->bulk_scratch); kfree(instance); - return -ENODEV; +err_disconnect_vchi: + vchi_disconnect(vchi_instance); + return err; } EXPORT_SYMBOL_GPL(vchiq_mmal_init); -- 2.7.4 From 8c1ed0f9b49eaaabd954266981c95c4cfd35bf49 Mon Sep 17 00:00:00 2001 From: Hoegeun Kwon Date: Mon, 26 Oct 2020 20:47:31 +0900 Subject: [PATCH 08/16] drm/vc4: drv: Add error handding for bind There is a problem that if vc4_drm bind fails, a memory leak occurs on the drm_property_create side. Add error handding for drm_mode_config. Change-Id: If23c0dabf01b85368871981f0cba427982922615 Signed-off-by: Hoegeun Kwon --- drivers/gpu/drm/vc4/vc4_drv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index d840bca..d0e70d15 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -328,6 +328,7 @@ unbind_all: component_unbind_all(dev, drm); gem_destroy: vc4_gem_destroy(drm); + drm_mode_config_cleanup(drm); vc4_bo_cache_destroy(drm); dev_put: drm_dev_put(drm); -- 2.7.4 From d29f5c328c92a8ff2c4f146348b8035e484f83c5 Mon Sep 17 00:00:00 2001 From: Seung-Woo Kim Date: Tue, 27 Oct 2020 19:23:56 +0900 Subject: [PATCH 09/16] brcmfmac: Fix memory leak for unpaired brcmf_{alloc/free} There are missig brcmf_free() for brcmf_alloc(). Fix memory leak by adding missed brcmf_free(). Change-Id: I050398a7b828b0fb2aadbe491f353b3d3c47bdd6 Reported-by: Jaehoon Chung Signed-off-by: Seung-Woo Kim --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 6 ++++-- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 3be60ae..cb68f54 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -1936,16 +1936,18 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id) fwreq = brcmf_pcie_prepare_fw_request(devinfo); if (!fwreq) { ret = -ENOMEM; - goto fail_bus; + goto fail_brcmf; } ret = brcmf_fw_get_firmwares(bus->dev, fwreq, brcmf_pcie_setup); if (ret < 0) { kfree(fwreq); - goto fail_bus; + goto fail_brcmf; } return 0; +fail_brcmf: + brcmf_free(&devinfo->pdev->dev); fail_bus: kfree(bus->msgbuf); kfree(bus); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index a3595b1..e6f4d4e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -4432,6 +4432,7 @@ void brcmf_sdio_remove(struct brcmf_sdio *bus) brcmf_sdiod_intr_unregister(bus->sdiodev); brcmf_detach(bus->sdiodev->dev); + brcmf_free(bus->sdiodev->dev); cancel_work_sync(&bus->datawork); if (bus->brcmf_wq) -- 2.7.4 From e256997d9da3c74449ed05bd2fbbc649a903c8b6 Mon Sep 17 00:00:00 2001 From: Sung-hun Kim Date: Tue, 27 Oct 2020 20:48:36 +0900 Subject: [PATCH 10/16] mm: LKSM: bug fix for kernel memory leak For efficiency, LKSM cleans exited processes in a batched manner when it finishes a scanning iteration. When it finds exited process while it is in the scanning iteration, it just pends the mm_slot of the exited process to the internal list. On the other hend, when KSM daemon cleans mm_slots of exited processes, it should care regions of exited processes to remove unreferenced lksm_region objects. Previously, most regions are maintained properly but only regions in "head" of the exited process list does not be cleaned due to the buggy implementation. At last, uncleaned objects are remained as unreferenced garbages. Follow message is detected by kmemleak (reported by Suengwoo Kim): ========================================================================= unreferenced object 0xffffff80c7083600 (size 128): comm "ksm_crawld", pid 41, jiffies 4294918362 (age 95.632s) hex dump (first 32 bytes): 00 37 08 c7 80 ff ff ff 60 82 19 bd 80 ff ff ff .7......`....... 00 35 08 c7 80 ff ff ff 00 00 00 00 00 00 00 00 .5.............. backtrace: [<0000000048313958>] kmem_cache_alloc_trace+0x1e0/0x348 [<00000000fd246822>] lksm_region_ref_append+0x48/0xf8 [<00000000c5a818a0>] ksm_join+0x3a0/0x498 [<00000000b2c3f36a>] lksm_prepare_full_scan+0xe8/0x390 [<00000000013943b5>] lksm_crawl_thread+0x214/0xbf8 [<00000000b4ce0593>] kthread+0x1b0/0x1b8 [<000000002a3f7216>] ret_from_fork+0x10/0x18 unreferenced object 0xffffff80c7083700 (size 128): comm "ksm_crawld", pid 41, jiffies 4294918362 (age 95.632s) hex dump (first 32 bytes): 00 39 08 c7 80 ff ff ff 00 36 08 c7 80 ff ff ff .9.......6...... 00 35 08 c7 80 ff ff ff 00 00 00 00 00 00 00 00 .5.............. backtrace: [<0000000048313958>] kmem_cache_alloc_trace+0x1e0/0x348 [<00000000fd246822>] lksm_region_ref_append+0x48/0xf8 [<00000000c5a818a0>] ksm_join+0x3a0/0x498 [<00000000b2c3f36a>] lksm_prepare_full_scan+0xe8/0x390 [<00000000013943b5>] lksm_crawl_thread+0x214/0xbf8 [<00000000b4ce0593>] kthread+0x1b0/0x1b8 [<000000002a3f7216>] ret_from_fork+0x10/0x18 ... ========================================================================= This patch takes care of such possible kernel memory leak problem. Change-Id: Ifb4963773b8803da239a1d3108c5b2690d22d531 Signed-off-by: Sung-hun Kim --- mm/lksm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/lksm.c b/mm/lksm.c index e6a1533..6081f4b 100644 --- a/mm/lksm.c +++ b/mm/lksm.c @@ -2836,6 +2836,9 @@ static void lksm_flush_removed_mm_list(void) cond_resched(); remove_trailing_rmap_items(head, &head->rmap_list); +#ifdef CONFIG_LKSM_FILTER + lksm_region_ref_list_release(head); +#endif clear_bit(MMF_VM_MERGEABLE, &head->mm->flags); mmdrop(head->mm); free_mm_slot(head); -- 2.7.4 From acea88dbb780d2415aa08108f5b9f87ee7db7a08 Mon Sep 17 00:00:00 2001 From: Sung-hun Kim Date: Wed, 28 Oct 2020 19:26:31 +0900 Subject: [PATCH 11/16] mm: LKSM: bug fix for KASAN out-of-bound access error on accessing a filter KASAN reports out-of-bound accesses (reported by Jaehoon Chung) on slab which is performed for obtaining a next filtered address to find a sharable page. LKSM exploits bitmap-based filters to find sharable pages in an efficient way. A buggy code is a kind of miscalculation for boundary of the allocated bitmap. This patch takes care of it. Change-Id: If45c5ce175db067523b60f11e69e12d2bc798659 Signed-off-by: Sung-hun Kim --- mm/lksm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mm/lksm.c b/mm/lksm.c index 6081f4b..3dcaacf 100644 --- a/mm/lksm.c +++ b/mm/lksm.c @@ -2917,7 +2917,7 @@ static void lksm_insert_mm_slot_ordered(struct mm_slot *slot) static inline void __lksm_copy_filter (unsigned long *orig, unsigned long *newer, int size) { - while (size-- >= 0) + while (--size >= 0) *(newer++) = *(orig++); } @@ -3012,8 +3012,8 @@ static inline unsigned long lksm_get_next_filtered_address unsigned long next_offset, curr_offset, nbits; curr_offset = (addr - base) >> PAGE_SHIFT; - nbits = (region->len == 0) ? BITS_PER_LONG : - (region->len << (6 + PAGE_SHIFT)); + nbits = region->len * BITS_PER_LONG; + if (region->len > SINGLE_FILTER_LEN) next_offset = find_next_bit(region->filter, nbits, curr_offset); else -- 2.7.4 From 36d582530bcbba2167da9c697adbda4047b3ffc6 Mon Sep 17 00:00:00 2001 From: Hoegeun Kwon Date: Tue, 3 Nov 2020 17:08:54 +0900 Subject: [PATCH 12/16] Revert "drm/vc4: kms: Fix VBLANK reporting on a disabled CRTC" This reverts commit e805316d5d44b1f1f080fd8ae8a34b69329d940c. Revert this patch for apply patch version 2. Change-Id: If9525aaa3835ab80b8ca83271dc584f68254018a Signed-off-by: Hoegeun Kwon --- drivers/gpu/drm/vc4/vc4_kms.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index a552221..06b291f 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -651,14 +651,6 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = { * need to consider all the running CRTCs in the DRM device to assign * a FIFO, not just the one in the state. * - * - To fix the above, we can't use drm_atomic_get_crtc_state on all - * enabled CRTCs to pull their CRTC state into the global state, since - * a page flip would start considering their vblank to complete. Since - * we don't have a guarantee that they are actually active, that - * vblank might never happen, and shouldn't even be considered if we - * want to do a page flip on a single CRTC. That can be tested by - * doing a modetest -v first on HDMI1 and then on HDMI0. - * * - Since we need the pixelvalve to be disabled and enabled back when * the FIFO is changed, we should keep the FIFO assigned for as long * as the CRTC is enabled, only considering it free again once that @@ -669,8 +661,8 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) { unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0); + struct vc4_dev *vc4 = to_vc4_dev(state->dev); struct drm_crtc_state *old_crtc_state, *new_crtc_state; - struct drm_crtc_state *crtc_state; struct drm_crtc *crtc; unsigned int i; @@ -682,13 +674,14 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, * the same CRTCs, instead of evaluating only the CRTC being * modified. */ - for_each_new_or_current_crtc_state(state, crtc, crtc_state) { - struct vc4_crtc_state *vc4_crtc_state; - if (!crtc_state->enable) + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { + struct drm_crtc_state *crtc_state; + if (!crtc->state->enable) continue; - vc4_crtc_state = to_vc4_crtc_state(crtc_state); - unassigned_channels &= ~BIT(vc4_crtc_state->assigned_channel); + crtc_state = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); } for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { @@ -704,8 +697,10 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev, if (!new_crtc_state->enable) continue; - if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED) + if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED) { + unassigned_channels &= ~BIT(new_vc4_crtc_state->assigned_channel); continue; + } /* * The problem we have to solve here is that we have -- 2.7.4 From 2578979a9fe4c962bf782fe65997843f9acc2bcd Mon Sep 17 00:00:00 2001 From: Hoegeun Kwon Date: Tue, 3 Nov 2020 17:08:57 +0900 Subject: [PATCH 13/16] Revert "drm/vc4: kms: Don't disable the muxing of an active CRTC" This reverts commit 5d2fec61a25bacc49ee8e84b3c19aee1522f7289. Revert this patch for apply patch version 2. Change-Id: Ib7ff9b242dc3f208cf58f7d8f0741321b725c657 Signed-off-by: Hoegeun Kwon --- drivers/gpu/drm/vc4/vc4_kms.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 06b291f..367cead 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -191,23 +191,6 @@ static void vc4_hvs_pv_muxing_commit(struct vc4_dev *vc4, } } -static struct drm_crtc_state * -drm_atomic_get_new_or_current_crtc_state(struct drm_atomic_state *state, - struct drm_crtc *crtc) -{ - struct drm_crtc_state *crtc_state; - - crtc_state = drm_atomic_get_new_crtc_state(state, crtc); - if (crtc_state) - return crtc_state; - - return crtc->state; -} - -#define for_each_new_or_current_crtc_state(__state, crtc, crtc_state) \ - list_for_each_entry(crtc, &__state->dev->mode_config.crtc_list, head) \ - for_each_if(crtc_state = drm_atomic_get_new_or_current_crtc_state(__state, crtc)) - static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4, struct drm_atomic_state *state) { @@ -217,16 +200,16 @@ static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4, unsigned char dsp3_mux = 3; unsigned char dsp4_mux = 3; unsigned char dsp5_mux = 3; + unsigned int i; u32 reg; - for_each_new_or_current_crtc_state(state, crtc, crtc_state) { + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state); struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); - struct vc4_crtc_state *vc4_state; if (!crtc_state->active) continue; - vc4_state = to_vc4_crtc_state(crtc_state); switch (vc4_crtc->data->hvs_output) { case 2: dsp2_mux = (vc4_state->assigned_channel == 2) ? 0 : 1; -- 2.7.4 From 75f8ae9a4394a202950001d7e6233e0a4cbb1b4d Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Tue, 3 Nov 2020 17:08:59 +0900 Subject: [PATCH 14/16] drm/vc4: kms: Remove useless define NUM_OUTPUTS isn't used anymore, let's remove it. Signed-off-by: Maxime Ripard [hoegeun.kwon: Needed to fix page flip issue of dual hdmi and for enable force hotplug configure.] Signed-off-by: Hoegeun Kwon Change-Id: I20b47a2da045094a06cef9e9a924f46c4dd1d4a9 --- drivers/gpu/drm/vc4/vc4_kms.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 367cead..261ac79 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -615,7 +615,6 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = { .atomic_destroy_state = vc4_load_tracker_destroy_state, }; -#define NUM_OUTPUTS 6 #define NUM_CHANNELS 3 /* -- 2.7.4 From 18a12e7b42d9a732fa402b739ebffd7dbe92cb39 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Tue, 3 Nov 2020 17:09:01 +0900 Subject: [PATCH 15/16] drm/vc4: kms: Rename NUM_CHANNELS The NUM_CHANNELS define has a pretty generic name and was right before the function using it. Let's move to something that makes the hardware-specific nature more obvious, and to a more appropriate place. Signed-off-by: Maxime Ripard [hoegeun.kwon: Needed to fix page flip issue of dual hdmi and for enable force hotplug configure.] Signed-off-by: Hoegeun Kwon Change-Id: Ib94c2b3ffbc7e5c95f2c3a965919e523c68f8a12 --- drivers/gpu/drm/vc4/vc4_kms.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index 261ac79..c87590d 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -27,6 +27,8 @@ #include "vc4_drv.h" #include "vc4_regs.h" +#define HVS_NUM_CHANNELS 3 + struct vc4_ctm_state { struct drm_private_state base; struct drm_color_ctm *ctm; @@ -615,8 +617,6 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = { .atomic_destroy_state = vc4_load_tracker_destroy_state, }; -#define NUM_CHANNELS 3 - /* * The BCM2711 HVS has up to 7 output connected to the pixelvalves and * the TXP (and therefore all the CRTCs found on that platform). @@ -642,7 +642,7 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = { static int vc4_pv_muxing_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) { - unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0); + unsigned long unassigned_channels = GENMASK(HVS_NUM_CHANNELS - 1, 0); struct vc4_dev *vc4 = to_vc4_dev(state->dev); struct drm_crtc_state *old_crtc_state, *new_crtc_state; struct drm_crtc *crtc; -- 2.7.4 From 29f6ebcf83fb4d1f1f407a9da191ccffc927cfd3 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Tue, 3 Nov 2020 17:09:12 +0900 Subject: [PATCH 16/16] drm/vc4: kms: Add functions to create the state objects We're going to add a new private state, so let's move the previous state function creation to some functions to make further additions easier. Signed-off-by: Maxime Ripard [hoegeun.kwon: Needed to fix page flip issue of dual hdmi and for enable force hotplug configure.] Signed-off-by: Hoegeun Kwon Change-Id: Ice57c56dfbdfbb81f486cba7626512cfbb12b905 --- drivers/gpu/drm/vc4/vc4_kms.c | 71 ++++++++++++++++++++++++++++++------------- 1 file changed, 50 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index c87590d..7db8cd6 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -98,6 +98,27 @@ static const struct drm_private_state_funcs vc4_ctm_state_funcs = { .atomic_destroy_state = vc4_ctm_destroy_state, }; +static int vc4_ctm_obj_init(struct vc4_dev *vc4) +{ + struct vc4_ctm_state *ctm_state; + + drm_modeset_lock_init(&vc4->ctm_state_lock); + + ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL); + if (!ctm_state) + return -ENOMEM; + + drm_atomic_private_obj_init(vc4->dev, &vc4->ctm_manager, &ctm_state->base, + &vc4_ctm_state_funcs); + + return 0; +} + +static void vc4_ctm_obj_fini(struct vc4_dev *vc4) +{ + drm_atomic_private_obj_fini(&vc4->ctm_manager); +} + /* Converts a DRM S31.32 value to the HW S0.9 format. */ static u16 vc4_ctm_s31_32_to_s0_9(u64 in) { @@ -617,6 +638,24 @@ static const struct drm_private_state_funcs vc4_load_tracker_state_funcs = { .atomic_destroy_state = vc4_load_tracker_destroy_state, }; +static int vc4_load_tracker_obj_init(struct vc4_dev *vc4) +{ + struct vc4_load_tracker_state *load_state; + + if (!vc4->load_tracker_available) + return 0; + + load_state = kzalloc(sizeof(*load_state), GFP_KERNEL); + if (!load_state) + return -ENOMEM; + + drm_atomic_private_obj_init(vc4->dev, &vc4->load_tracker, + &load_state->base, + &vc4_load_tracker_state_funcs); + + return 0; +} + /* * The BCM2711 HVS has up to 7 output connected to the pixelvalves and * the TXP (and therefore all the CRTCs found on that platform). @@ -756,8 +795,6 @@ static const struct drm_mode_config_funcs vc4_mode_funcs = { int vc4_kms_load(struct drm_device *dev) { struct vc4_dev *vc4 = to_vc4_dev(dev); - struct vc4_ctm_state *ctm_state; - struct vc4_load_tracker_state *load_state; int ret; if (!of_device_is_compatible(dev->dev->of_node, "brcm,bcm2711-vc5")) { @@ -795,30 +832,22 @@ int vc4_kms_load(struct drm_device *dev) dev->mode_config.allow_fb_modifiers = true; dev->mode_config.normalize_zpos = true; - drm_modeset_lock_init(&vc4->ctm_state_lock); - - ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL); - if (!ctm_state) - return -ENOMEM; - - drm_atomic_private_obj_init(dev, &vc4->ctm_manager, &ctm_state->base, - &vc4_ctm_state_funcs); - - if (vc4->load_tracker_available) { - load_state = kzalloc(sizeof(*load_state), GFP_KERNEL); - if (!load_state) { - drm_atomic_private_obj_fini(&vc4->ctm_manager); - return -ENOMEM; - } + ret = vc4_ctm_obj_init(vc4); + if (ret) + return ret; - drm_atomic_private_obj_init(dev, &vc4->load_tracker, - &load_state->base, - &vc4_load_tracker_state_funcs); - } + ret = vc4_load_tracker_obj_init(vc4); + if (ret) + goto ctm_fini; drm_mode_config_reset(dev); drm_kms_helper_poll_init(dev); return 0; + +ctm_fini: + vc4_ctm_obj_fini(vc4); + + return ret; } -- 2.7.4