| Message ID | 20190503122949.12266-21-tomi.valkeinen@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/bridge: tc358767: DP support | expand |
On 03.05.2019 14:29, Tomi Valkeinen wrote: > In tc_bridge_mode_set callback, we store the pointer to the given > drm_display_mode, and use the mode later. Storing a pointer in such a > way looks very suspicious to me, and I have observed odd issues where > the timings were apparently (at least mostly) zero. > > Do a copy of the drm_display_mode instead to ensure we don't refer to > freed/modified data. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> -- Regards Andrzej > --- > drivers/gpu/drm/bridge/tc358767.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index 591bf64ac916..8f6d601def3f 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -205,7 +205,7 @@ struct tc_data { > /* display edid */ > struct edid *edid; > /* current mode */ > - const struct drm_display_mode *mode; > + struct drm_display_mode mode; > > u32 rev; > u8 assr; > @@ -1032,12 +1032,12 @@ static int tc_stream_enable(struct tc_data *tc) > /* PXL PLL setup */ > if (tc_test_pattern) { > ret = tc_pxl_pll_en(tc, clk_get_rate(tc->refclk), > - 1000 * tc->mode->clock); > + 1000 * tc->mode.clock); > if (ret) > goto err; > } > > - ret = tc_set_video_mode(tc, tc->mode); > + ret = tc_set_video_mode(tc, &tc->mode); > if (ret) > return ret; > > @@ -1180,7 +1180,7 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge, > { > struct tc_data *tc = bridge_to_tc(bridge); > > - tc->mode = mode; > + tc->mode = *mode; > } > > static int tc_connector_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 591bf64ac916..8f6d601def3f 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -205,7 +205,7 @@ struct tc_data { /* display edid */ struct edid *edid; /* current mode */ - const struct drm_display_mode *mode; + struct drm_display_mode mode; u32 rev; u8 assr; @@ -1032,12 +1032,12 @@ static int tc_stream_enable(struct tc_data *tc) /* PXL PLL setup */ if (tc_test_pattern) { ret = tc_pxl_pll_en(tc, clk_get_rate(tc->refclk), - 1000 * tc->mode->clock); + 1000 * tc->mode.clock); if (ret) goto err; } - ret = tc_set_video_mode(tc, tc->mode); + ret = tc_set_video_mode(tc, &tc->mode); if (ret) return ret; @@ -1180,7 +1180,7 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge, { struct tc_data *tc = bridge_to_tc(bridge); - tc->mode = mode; + tc->mode = *mode; } static int tc_connector_get_modes(struct drm_connector *connector)
In tc_bridge_mode_set callback, we store the pointer to the given drm_display_mode, and use the mode later. Storing a pointer in such a way looks very suspicious to me, and I have observed odd issues where the timings were apparently (at least mostly) zero. Do a copy of the drm_display_mode instead to ensure we don't refer to freed/modified data. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/bridge/tc358767.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)