Hi Boris,
On Mon, 2020-01-27 at 12:00 +0100, Boris Brezillon wrote:
Now that bridges can expose the bus format/flags they expect, we can use those instead of the relying on the display_info provided by the connector (which is only valid if the encoder is directly connected to bridge element driving the panel/display).
We also explicitly expose the bus formats supported by our encoder by filling encoder->output_bus_caps with proper info.
Signed-off-by: Boris Brezillon boris.brezillon@collabora.com
Changes in v7:
- Add an imx_pd_format_supported() helper (suggested by Philipp)
- Simplify imx_pd_bridge_atomic_get_output_bus_fmts() (suggested by Philipp)
- Simplify imx_pd_bridge_atomic_get_input_bus_fmts()
- Explicitly set the duplicate/destro_state() and reset() hooks
Changes in v3 (all suggested by Philipp):
- Adjust to match core changes
- Propagate output format to input format
- Pick a default value when output_fmt = _FIXED
- Add missing BGR888 and GBR888 fmts to imx_pd_bus_fmts[]
Changes in v2:
- Adjust things to match the new bus-format negotiation infra
drivers/gpu/drm/imx/parallel-display.c | 176 +++++++++++++++++++++---- 1 file changed, 151 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index 3dca424059f7..031579aa7fc2 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -24,6 +24,7 @@
[...]
+static u32 * +imx_pd_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
struct drm_bridge_state *bridge_state,struct drm_crtc_state *crtc_state,struct drm_connector_state *conn_state,u32 output_fmt,unsigned int *num_input_fmts)+{
- struct imx_parallel_display *imxpd = bridge_to_imxpd(bridge);
- u32 *input_fmts;
- /*
* If the next bridge does not support bus format negotiation, let's* use the static bus format definition (imxpd->bus_format) if it's* specified, RGB888 when it's not.*/- if (output_fmt == MEDIA_BUS_FMT_FIXED)
output_fmt = imxpd->bus_format ? : MEDIA_BUS_FMT_RGB888_1X24;- /* Now make sure the requested output format is supported. */
- if ((imxpd->bus_format && imxpd->bus_format != output_fmt) ||
!imx_pd_format_supported(output_fmt)) {*num_input_fmts = 0;return NULL;- }
- *num_input_fmts = 1;
- input_fmts = kcalloc(*num_input_fmts, sizeof(*input_fmts),
GFP_KERNEL);
You could just use kzalloc() here.
- if (!input_fmts)
return NULL;- input_fmts[0] = output_fmt;
- return input_fmts;
+}
+static int imx_pd_bridge_atomic_check(struct drm_bridge *bridge,
struct drm_bridge_state *bridge_state,struct drm_crtc_state *crtc_state,struct drm_connector_state *conn_state)+{
- struct imx_crtc_state *imx_crtc_state = to_imx_crtc_state(crtc_state);
- struct drm_display_info *di = &conn_state->connector->display_info;
- struct imx_parallel_display *imxpd = bridge_to_imxpd(bridge);
- struct drm_bridge_state *next_bridge_state = NULL;
- struct drm_bridge *next_bridge;
- u32 bus_flags, bus_fmt;
- next_bridge = drm_bridge_get_next_bridge(bridge);
- if (next_bridge)
next_bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state,next_bridge);- if (next_bridge_state)
bus_flags = next_bridge_state->input_bus_cfg.flags;- else if (!imxpd->bus_format && di->num_bus_formats)
bus_flags = di->bus_flags;- else
bus_flags = imxpd->bus_flags;- bus_fmt = bridge_state->input_bus_cfg.format;
- if (!imx_pd_format_supported(bus_fmt))
return -EINVAL;- if (bus_flags &
~(DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_DE_HIGH |DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE))
Maybe add a warning so the first person to hook up a bridge / panel with unsupported flags knows where to look.
Otherwise this looks fine to me.
Reviewed-by: Philipp Zabel p.zabel@pengutronix.de
regards Philipp