Hi Liu,
thank you for your comments.
Am Freitag, den 22.07.2016, 12:01 +0800 schrieb Ying Liu:
Hi Philipp,
This patch's headline doesn't exactly reflect what the patch actually does - retrieve lvds bus format from imx_crtc_state during encoder mode_set.
Yes. I initially stored ldb_bus_format in imx_ldb_ch, then fixed that, and forgot to change the subject.
On Thu, Jul 21, 2016 at 9:25 PM, Philipp Zabel p.zabel@pengutronix.de wrote:
The code in imx_ldb_encoder_mode_set crashes trying to access the crtc->state->state drm_atomic_state pointer if that was previously cleared by drm_atomic_helper_swap_state.
Nit: Providing a crash log might be good.
Will do.
Instead of trying to walk all connectors during encoder mode_set to determine the LVDS bus format from panel or external bridge connector display info, store it in imx_crtc_state during encoder atomic_check, where it is already known, and just retrieve it from imx_crtc_state during encoder mode_set.
Fixes: 49f98bc4d44a4 ("drm/imx: store internal bus configuration in crtc state") Signed-off-by: Philipp Zabel p.zabel@pengutronix.de
drivers/gpu/drm/imx/imx-drm.h | 1 + drivers/gpu/drm/imx/imx-ldb.c | 22 +++++----------------- 2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/imx/imx-drm.h b/drivers/gpu/drm/imx/imx-drm.h index bdaa381..5e2f68a 100644 --- a/drivers/gpu/drm/imx/imx-drm.h +++ b/drivers/gpu/drm/imx/imx-drm.h @@ -19,6 +19,7 @@ struct imx_crtc_state { u32 bus_flags; int di_hsync_pin; int di_vsync_pin;
u32 ldb_bus_format;Pretty much a hack here, I think. Comparing to the other members of imx_crtc_state, ldb_bus_format is less likely to be a member, since crtc just doesn't like to know the LVDS bus format...
I suppose I should keep the connector walk then. I'll send a new version.
[...]
Instead of introducing imx_crtc_state->ldb_bus_format to get the lvds bus format, you may get the connector via crtc_state->connector_mask(verify connector->encoder to be the encoder we are talking about). connector_mask is maintained well by the atomic core I assume.
I could also use drm_for_each_connector and just keep the (connector->state->crtc == encoder->crtc) check in the loop.
regards Philipp