Hi Boris,
Thank you for the patch.
On Wed, Oct 23, 2019 at 05:45:07PM +0200, Boris Brezillon wrote:
Some LVDS encoder might support several input/output bus formats. Add
s/encoder/encoders/
a way to describe the one used on a specific design by adding optional
s/the one/the format/
'data-mapping' properties to the input/output ports.
Signed-off-by: Boris Brezillon boris.brezillon@collabora.com
Changes in v3:
- Use bus-width for the rgb24/rgb18 distinction
- Adjust code to match core changes
- Get rid of of_get_data_mapping()
- Don't implement ->atomic_check() (the core now takes care of bus flags propagation)
Changes in v2:
- Make the bus-format negotiation logic more generic
drivers/gpu/drm/bridge/lvds-encoder.c | 72 +++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c index e2132a8d5106..a2a8f7f4ac97 100644 --- a/drivers/gpu/drm/bridge/lvds-encoder.c +++ b/drivers/gpu/drm/bridge/lvds-encoder.c @@ -6,6 +6,7 @@ #include <linux/gpio/consumer.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/of_device.h> #include <linux/of_graph.h> #include <linux/platform_device.h>
@@ -16,6 +17,8 @@ struct lvds_encoder { struct drm_bridge bridge; struct drm_bridge *panel_bridge; struct gpio_desc *powerdown_gpio;
- u32 output_fmt;
- u32 input_fmt;
};
static int lvds_encoder_attach(struct drm_bridge *bridge) @@ -48,10 +51,40 @@ static void lvds_encoder_disable(struct drm_bridge *bridge) gpiod_set_value_cansleep(lvds_encoder->powerdown_gpio, 1); }
+static u32 *lvds_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 lvds_encoder *lvds_encoder = container_of(bridge,
struct lvds_encoder,bridge);- u32 *input_fmts;
- if (output_fmt == MEDIA_BUS_FMT_FIXED ||
output_fmt == lvds_encoder->output_fmt)*num_input_fmts = 1;- else
*num_input_fmts = 0;- if (!*num_input_fmts)
return NULL;- input_fmts = kcalloc(*num_input_fmts, sizeof(*input_fmts), GFP_KERNEL);
As *num_input_fmts == 1, you can
input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
- if (!input_fmts)
return NULL;- input_fmts[0] = lvds_encoder->input_fmt;
- return input_fmts;
+}
static struct drm_bridge_funcs funcs = { .attach = lvds_encoder_attach, .enable = lvds_encoder_enable, .disable = lvds_encoder_disable,
- .atomic_get_input_bus_fmts = lvds_atomic_get_input_bus_fmts,
};
static int lvds_encoder_probe(struct platform_device *pdev) @@ -62,11 +95,16 @@ static int lvds_encoder_probe(struct platform_device *pdev) struct device_node *panel_node; struct drm_panel *panel; struct lvds_encoder *lvds_encoder;
const char *output_data_mapping = NULL;
u32 input_bus_width = 0;
lvds_encoder = devm_kzalloc(dev, sizeof(*lvds_encoder), GFP_KERNEL); if (!lvds_encoder) return -ENOMEM;
lvds_encoder->input_fmt = MEDIA_BUS_FMT_FIXED;
lvds_encoder->output_fmt = MEDIA_BUS_FMT_FIXED;
lvds_encoder->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown", GPIOD_OUT_HIGH); if (IS_ERR(lvds_encoder->powerdown_gpio)) {
@@ -77,6 +115,25 @@ static int lvds_encoder_probe(struct platform_device *pdev) return err; }
- port = of_graph_get_port_by_id(dev->of_node, 0);
- if (!port) {
dev_dbg(dev, "port 0 not found\n");return -ENXIO;- }
- of_node_put(port);
- if (of_property_read_u32(port, "bus-width", &input_bus_width)) {
You're using port after calling of_node_put().
lvds_encoder->input_fmt = MEDIA_BUS_FMT_FIXED;- } else if (input_bus_width == 18) {
lvds_encoder->input_fmt = MEDIA_BUS_FMT_RGB666_1X18;- } else if (input_bus_width == 24) {
lvds_encoder->input_fmt = MEDIA_BUS_FMT_RGB888_1X24;- } else {
dev_dbg(dev, "unsupported bus-width)\n");
s/bus-width)/bus-width/
but I think you should make the error message a bit clearer
dev_dbg(dev, "unsupported bus-width value %u on port 0\n", input_bus_width);
return -ENOTSUPP;- }
- /* Locate the panel DT node. */ port = of_graph_get_port_by_id(dev->of_node, 1); if (!port) {
@@ -84,6 +141,21 @@ static int lvds_encoder_probe(struct platform_device *pdev) return -ENXIO; }
- of_property_read_string(port, "data-mapping", &output_data_mapping);
- if (!output_data_mapping) {
lvds_encoder->output_fmt = MEDIA_BUS_FMT_FIXED;- } else if (!strcmp(output_data_mapping, "jeida-18")) {
lvds_encoder->output_fmt = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG;- } else if (!strcmp(output_data_mapping, "jeida-24")) {
lvds_encoder->output_fmt = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;- } else if (!strcmp(output_data_mapping, "vesa-24")) {
lvds_encoder->output_fmt = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;- } else {
of_node_put(port);dev_dbg(dev, "unsupported output data-mapping\n");return -ENOTSUPP;- }
- endpoint = of_get_child_by_name(port, "endpoint"); of_node_put(port); if (!endpoint) {