| Message ID | 20220628181838.2031-5-max.oss.09@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panel: simple: add bus-format support for panel-dpi | expand |
On 28-Jun-22 23:48, Max Krummenacher wrote: > From: Max Krummenacher <max.krummenacher@toradex.com> > > Evaluate the device tree bus-format property to set bus_format for > a 'panel-dpi' panel. Additionally infer the bpc value from the > given bus-format. > > Valid values for bus-format are found in: > <include/dt-bindings/display/dt-media-bus-format.h> > > This completes the addition of panel-dpi to completely specify > a panel-simple panel from the device tree. > > Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com> > > --- > > Changes in v3: > - Moved the bus-format property under the port/endpoint node as > suggested by Rob Herring > > Changes in v2: > - Fix errors found by dt_binding_check > > drivers/gpu/drm/panel/panel-simple.c | 49 ++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index 4a2e580a2f7b..f1a457f1069e 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -21,9 +21,11 @@ > * DEALINGS IN THE SOFTWARE. > */ > > +#include <dt-bindings/display/dt-media-bus-format.h> > #include <linux/delay.h> > #include <linux/gpio/consumer.h> > #include <linux/module.h> > +#include <linux/of_graph.h> > #include <linux/of_platform.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > @@ -449,10 +451,12 @@ static int panel_dpi_probe(struct device *dev, > struct panel_simple *panel) > { > struct display_timing *timing; > + struct device_node *endpoint; > const struct device_node *np; > struct panel_desc *desc; > unsigned int bus_flags; > struct videomode vm; > + u32 bus_format; > int ret; > > np = dev->of_node; > @@ -477,6 +481,51 @@ static int panel_dpi_probe(struct device *dev, > of_property_read_u32(np, "width-mm", &desc->size.width); > of_property_read_u32(np, "height-mm", &desc->size.height); > > + endpoint = of_graph_get_endpoint_by_regs(np, -1, -1); > + if (endpoint && > + !of_property_read_u32(endpoint, "bus-format", &bus_format)) { > + /* infer bpc from bus-format */ > + switch (bus_format) { > + case DT_MEDIA_BUS_FMT_RGB565_1X16: > + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; > + desc->bpc = 6; > + break; > + case DT_MEDIA_BUS_FMT_RGB666_1X18: > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; > + desc->bpc = 6; > + break; > + case DT_MEDIA_BUS_FMT_RGB666_1X24_CPADHI: > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; > + desc->bpc = 6; > + break; > + case DT_MEDIA_BUS_FMT_BGR888_1X24: > + desc->bus_format = MEDIA_BUS_FMT_BGR888_1X24; > + desc->bpc = 8; > + break; > + case DT_MEDIA_BUS_FMT_GBR888_1X24: > + desc->bus_format = MEDIA_BUS_FMT_GBR888_1X24; > + desc->bpc = 8; > + break; > + case DT_MEDIA_BUS_FMT_RBG888_1X24: > + desc->bus_format = MEDIA_BUS_FMT_RBG888_1X24; > + desc->bpc = 8; > + break; > + case DT_MEDIA_BUS_FMT_RGB888_1X24: > + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; > + desc->bpc = 8; > + break; > + case DT_MEDIA_BUS_FMT_RGB888_1X32_PADHI: > + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X32_PADHI; > + desc->bpc = 8; > + break; > + default: > + dev_err(dev, "%pOF: unknown bus-format property\n", np); > + return -EINVAL; > + } > + } > + > + of_node_put(endpoint); > + > /* Extract bus_flags from display_timing */ > bus_flags = 0; > vm.flags = timing->flags; I understand that it is important to add a bus-format property for dumb dpi-panels, and I agree with the implementation in the patch-set. However, I do not yet fully understand Rob's comments on the dt-bindings side of patch set (patch 1/4) and what consequences it may cause if that remains unresolved. Given that the bus-format property gets added, I do not see any concern with the panel-simple driver patch. Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com> Regards Aradhya
Hi Aradhya Thanks for the review. Rob's comment requires changes to panel-common.yaml in order for this to get in. I think I know what needs to be done. However, as there is no agreement on bus-format in the first place there is no point in sorting that out now. Regards Max On Wed, Aug 3, 2022 at 8:45 AM Aradhya Bhatia <a-bhatia1@ti.com> wrote: > > > > On 28-Jun-22 23:48, Max Krummenacher wrote: > > From: Max Krummenacher <max.krummenacher@toradex.com> > > > > Evaluate the device tree bus-format property to set bus_format for > > a 'panel-dpi' panel. Additionally infer the bpc value from the > > given bus-format. > > > > Valid values for bus-format are found in: > > <include/dt-bindings/display/dt-media-bus-format.h> > > > > This completes the addition of panel-dpi to completely specify > > a panel-simple panel from the device tree. > > > > Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com> > > > > --- > > > > Changes in v3: > > - Moved the bus-format property under the port/endpoint node as > > suggested by Rob Herring > > > > Changes in v2: > > - Fix errors found by dt_binding_check > > > > drivers/gpu/drm/panel/panel-simple.c | 49 ++++++++++++++++++++++++++++ > > 1 file changed, 49 insertions(+) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > > index 4a2e580a2f7b..f1a457f1069e 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -21,9 +21,11 @@ > > * DEALINGS IN THE SOFTWARE. > > */ > > > > +#include <dt-bindings/display/dt-media-bus-format.h> > > #include <linux/delay.h> > > #include <linux/gpio/consumer.h> > > #include <linux/module.h> > > +#include <linux/of_graph.h> > > #include <linux/of_platform.h> > > #include <linux/platform_device.h> > > #include <linux/pm_runtime.h> > > @@ -449,10 +451,12 @@ static int panel_dpi_probe(struct device *dev, > > struct panel_simple *panel) > > { > > struct display_timing *timing; > > + struct device_node *endpoint; > > const struct device_node *np; > > struct panel_desc *desc; > > unsigned int bus_flags; > > struct videomode vm; > > + u32 bus_format; > > int ret; > > > > np = dev->of_node; > > @@ -477,6 +481,51 @@ static int panel_dpi_probe(struct device *dev, > > of_property_read_u32(np, "width-mm", &desc->size.width); > > of_property_read_u32(np, "height-mm", &desc->size.height); > > > > + endpoint = of_graph_get_endpoint_by_regs(np, -1, -1); > > + if (endpoint && > > + !of_property_read_u32(endpoint, "bus-format", &bus_format)) { > > + /* infer bpc from bus-format */ > > + switch (bus_format) { > > + case DT_MEDIA_BUS_FMT_RGB565_1X16: > > + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; > > + desc->bpc = 6; > > + break; > > + case DT_MEDIA_BUS_FMT_RGB666_1X18: > > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; > > + desc->bpc = 6; > > + break; > > + case DT_MEDIA_BUS_FMT_RGB666_1X24_CPADHI: > > + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; > > + desc->bpc = 6; > > + break; > > + case DT_MEDIA_BUS_FMT_BGR888_1X24: > > + desc->bus_format = MEDIA_BUS_FMT_BGR888_1X24; > > + desc->bpc = 8; > > + break; > > + case DT_MEDIA_BUS_FMT_GBR888_1X24: > > + desc->bus_format = MEDIA_BUS_FMT_GBR888_1X24; > > + desc->bpc = 8; > > + break; > > + case DT_MEDIA_BUS_FMT_RBG888_1X24: > > + desc->bus_format = MEDIA_BUS_FMT_RBG888_1X24; > > + desc->bpc = 8; > > + break; > > + case DT_MEDIA_BUS_FMT_RGB888_1X24: > > + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; > > + desc->bpc = 8; > > + break; > > + case DT_MEDIA_BUS_FMT_RGB888_1X32_PADHI: > > + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X32_PADHI; > > + desc->bpc = 8; > > + break; > > + default: > > + dev_err(dev, "%pOF: unknown bus-format property\n", np); > > + return -EINVAL; > > + } > > + } > > + > > + of_node_put(endpoint); > > + > > /* Extract bus_flags from display_timing */ > > bus_flags = 0; > > vm.flags = timing->flags; > > I understand that it is important to add a bus-format property for dumb > dpi-panels, and I agree with the implementation in the patch-set. > > However, > I do not yet fully understand Rob's comments on the dt-bindings side of > patch set (patch 1/4) and what consequences it may cause if that remains > unresolved. > > Given that the bus-format property gets added, I do not see any concern > with the panel-simple driver patch. > > > Reviewed-by: Aradhya Bhatia <a-bhatia1@ti.com> > > > Regards > Aradhya
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 4a2e580a2f7b..f1a457f1069e 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -21,9 +21,11 @@ * DEALINGS IN THE SOFTWARE. */ +#include <dt-bindings/display/dt-media-bus-format.h> #include <linux/delay.h> #include <linux/gpio/consumer.h> #include <linux/module.h> +#include <linux/of_graph.h> #include <linux/of_platform.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> @@ -449,10 +451,12 @@ static int panel_dpi_probe(struct device *dev, struct panel_simple *panel) { struct display_timing *timing; + struct device_node *endpoint; const struct device_node *np; struct panel_desc *desc; unsigned int bus_flags; struct videomode vm; + u32 bus_format; int ret; np = dev->of_node; @@ -477,6 +481,51 @@ static int panel_dpi_probe(struct device *dev, of_property_read_u32(np, "width-mm", &desc->size.width); of_property_read_u32(np, "height-mm", &desc->size.height); + endpoint = of_graph_get_endpoint_by_regs(np, -1, -1); + if (endpoint && + !of_property_read_u32(endpoint, "bus-format", &bus_format)) { + /* infer bpc from bus-format */ + switch (bus_format) { + case DT_MEDIA_BUS_FMT_RGB565_1X16: + desc->bus_format = MEDIA_BUS_FMT_RGB565_1X16; + desc->bpc = 6; + break; + case DT_MEDIA_BUS_FMT_RGB666_1X18: + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X18; + desc->bpc = 6; + break; + case DT_MEDIA_BUS_FMT_RGB666_1X24_CPADHI: + desc->bus_format = MEDIA_BUS_FMT_RGB666_1X24_CPADHI; + desc->bpc = 6; + break; + case DT_MEDIA_BUS_FMT_BGR888_1X24: + desc->bus_format = MEDIA_BUS_FMT_BGR888_1X24; + desc->bpc = 8; + break; + case DT_MEDIA_BUS_FMT_GBR888_1X24: + desc->bus_format = MEDIA_BUS_FMT_GBR888_1X24; + desc->bpc = 8; + break; + case DT_MEDIA_BUS_FMT_RBG888_1X24: + desc->bus_format = MEDIA_BUS_FMT_RBG888_1X24; + desc->bpc = 8; + break; + case DT_MEDIA_BUS_FMT_RGB888_1X24: + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X24; + desc->bpc = 8; + break; + case DT_MEDIA_BUS_FMT_RGB888_1X32_PADHI: + desc->bus_format = MEDIA_BUS_FMT_RGB888_1X32_PADHI; + desc->bpc = 8; + break; + default: + dev_err(dev, "%pOF: unknown bus-format property\n", np); + return -EINVAL; + } + } + + of_node_put(endpoint); + /* Extract bus_flags from display_timing */ bus_flags = 0; vm.flags = timing->flags;