Hi Ahmad.
> On 2/1/19 22:37, Sam Ravnborg wrote:
> > The problem with the RED/BLUE lines swapped is something I
> > have encountered while working with DRM support for Atmel at91sam9263 too.
> >
> > The solution selected is to extend the endpoint with
> > a new optional property:
> >
> > - wiring: Wiring of data lines to display.
> > "straight" - normal wiring.
> > "red-blue-reversed" - red and blue lines reversed.
> >
> > (media/video-interfaces.txt)
> >
> >
> > The DT node looks like this:
> >
> > port@0 {
> > reg = <0>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > lcdc_panel_output: endpoint@0 {
> > reg = <0>;
> > wiring = "red-blue-reversed";
> > remote-endpoint = <&panel_input>;
> > };
> > };
> >
> > This allows us to specify the swapping in the endpoint and
> > not in the panel.
> > So we can use the same panel, with the same bus_format, in several
> > designs some with red-blue swapped (reversed), and some not.
>
> A colleague suggested a property in the endpoint as well, but I shied
> away because of the extra hassle. Seems there's won't be a way around it ^^'..
>
> How do you intend to propagate this different wiring setting?
The way I have it implmented is more or less like this:
First find the wiring property:
1) Look up endpoint using of_graph_get_endpoint_by_regs()
2) Get wiring property
3) of_node_put(endpoint);
And then find and attach the panel:
4) drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, &bridge);
5) devm_drm_panel_bridge_add(dev, panel, DRM_MODE_CONNECTOR_DPI);
6) Then based on the wiring property I adjust bus_format
7) drm_simple_display_pipe_init()
8) drm_simple_display_pipe_attach_bridge()
But this is all virgin code that for now can build,
but has not yet seen any testing.
It is a lot of boilerplate for something relatively simple
and I hope there are ways to simplify this.
Relevant parts of the file pasted below.
But the translation of bus_format in a central place may prove a bit
difficult and I assume this as something that can differ
a lot between different HW solutions.
> How about having drm_of_find_panel_or_bridge adjust the
> (*panel)->connector->display_info.bus_formats array to account for the
> different wiring? That way there shouldn't be any need to adjust drivers.
But if you prove me wrong and this fly I am all for it.
Keep in mind that I am novice in the DRM land. So there may be better ways to do it.
Sam
static int lcdc_get_of_wiring(struct lcdc *lcdc,
const struct device_node *ep)
{
const char *str;
int ret;
ret = of_property_read_string(ep, "wiring", &str);
if (ret)
return ret;
if (strcmp(str, "red-green-reversed") == 0) {
lcdc->wiring_reversed = true;
} else if (strcmp(str, "straight") == 0) {
/* Use default format */
} else {
DRM_DEV_ERROR(lcdc->dev, "unknown \"wiring\" property: %s",
str);
return -EINVAL;
}
return 0;
}
static int lcdc_display_init(struct lcdc *lcdc, struct drm_device *drm)
{
struct drm_display_info *display_info;
const u32 *formats;
size_t nformats;
int ret;
display_info = &lcdc->panel->connector->display_info;
if (!display_info->num_bus_formats || !display_info->bus_formats) {
DRM_DEV_ERROR(lcdc->dev, "missing bus_format from panel");
return -EINVAL;
}
switch (display_info->bus_formats[0]) {
case MEDIA_BUS_FMT_RGB888_1X24:
case MEDIA_BUS_FMT_RGB565_1X16:
lcdc->bus_format = display_info->bus_formats[0];
break;
default:
DRM_DEV_ERROR(lcdc->dev, "unsupported bus_format: %d",
display_info->bus_formats[0]);
return -EINVAL;
}
/* Select formats depending on wiring (from bus_formats + from DT) */
if (lcdc->bus_format == MEDIA_BUS_FMT_RGB888_1X24) {
if (!lcdc->wiring_reversed) {
formats = bgr_formats_24;
nformats = ARRAY_SIZE(bgr_formats_24);
} else {
formats = rgb_formats_24;
nformats = ARRAY_SIZE(rgb_formats_24);
}
} else {
if (!lcdc->wiring_reversed) {
formats = bgr_formats_16;
nformats = ARRAY_SIZE(bgr_formats_16);
} else {
formats = rgb_formats_16;
nformats = ARRAY_SIZE(rgb_formats_16);
}
}
ret = drm_simple_display_pipe_init(drm, &lcdc->pipe,
&lcdc_display_funcs,
formats, nformats,
NULL, &lcdc->connector);
if (ret < 0)
DRM_DEV_ERROR(lcdc->dev, "failed to init display pipe: %d",
ret);
return ret;
}
static int lcdc_attach_panel(struct lcdc *lcdc, struct drm_device *drm)
{
struct drm_bridge *bridge;
struct drm_panel *panel;
struct device *dev;
int ret;
dev = lcdc->dev;
ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &panel, &bridge);
if (ret < 0)
return ret;
if (panel) {
lcdc->panel = panel;
bridge = devm_drm_panel_bridge_add(dev, panel,
DRM_MODE_CONNECTOR_DPI);
ret = PTR_ERR_OR_ZERO(bridge);
if (ret < 0) {
DRM_DEV_ERROR(dev, "failed to add bridge: %d", ret);
return ret;
}
} else {
DRM_DEV_ERROR(dev, "the bridge is not a panel");
return -ENODEV;
}
ret = lcdc_display_init(lcdc, drm);
if (ret < 0)
return ret;
ret = drm_simple_display_pipe_attach_bridge(&lcdc->pipe, bridge);
if (ret < 0)
DRM_DEV_ERROR(dev, "failed to attach bridge: %d", ret);
return ret;
}
static int lcdc_create_output(struct lcdc *lcdc, struct drm_device *drm)
{
struct device_node *endpoint;
int ret;
/* port@0/endpoint@0 is the only port/endpoint */
endpoint = of_graph_get_endpoint_by_regs(drm->dev->of_node, 0, 0);
if (!endpoint) {
DRM_DEV_ERROR(lcdc->dev, "failed to find endpoint node");
return -ENODEV;
}
lcdc_get_of_wiring(lcdc, endpoint);
of_node_put(endpoint);
ret = lcdc_attach_panel(lcdc, drm);
return ret;
}
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
