Panel initialization registers are per-display and not tied to the
controller itself. Different panels will specify their own registers.
Attach the sequences to the panel info struct so future panels
can specify their own sequences.
Signed-off-by: John Watts <contact@jookia.org>
---
.../gpu/drm/panel/panel-newvision-nv3052c.c | 25 ++++++++++++-------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
index 307335d0f1fc..b2ad9b3a5eb7 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -20,11 +20,18 @@
#include <drm/drm_modes.h>
#include <drm/drm_panel.h>
+struct nv3052c_reg {
+ u8 cmd;
+ u8 val;
+};
+
struct nv3052c_panel_info {
const struct drm_display_mode *display_modes;
unsigned int num_modes;
u16 width_mm, height_mm;
u32 bus_format, bus_flags;
+ const struct nv3052c_reg *panel_regs;
+ int panel_regs_len;
};
struct nv3052c {
@@ -36,12 +43,7 @@ struct nv3052c {
struct gpio_desc *reset_gpio;
};
-struct nv3052c_reg {
- u8 cmd;
- u8 val;
-};
-
-static const struct nv3052c_reg nv3052c_panel_regs[] = {
+static const struct nv3052c_reg ltk035c5444t_panel_regs[] = {
// EXTC Command set enable, select page 1
{ 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x01 },
// Mostly unknown registers
@@ -244,6 +246,7 @@ static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
static int nv3052c_prepare(struct drm_panel *panel)
{
struct nv3052c *priv = to_nv3052c(panel);
+ const struct nv3052c_reg *panel_regs = priv->panel_info->panel_regs;
struct mipi_dbi *dbi = &priv->dbi;
unsigned int i;
int err;
@@ -260,9 +263,11 @@ static int nv3052c_prepare(struct drm_panel *panel)
gpiod_set_value_cansleep(priv->reset_gpio, 0);
msleep(150);
- for (i = 0; i < ARRAY_SIZE(nv3052c_panel_regs); i++) {
- err = mipi_dbi_command(dbi, nv3052c_panel_regs[i].cmd,
- nv3052c_panel_regs[i].val);
+ int panel_regs_len = priv->panel_info->panel_regs_len;
+
+ for (i = 0; i < panel_regs_len; i++) {
+ err = mipi_dbi_command(dbi, panel_regs[i].cmd,
+ panel_regs[i].val);
if (err) {
dev_err(priv->dev, "Unable to set register: %d\n", err);
@@ -466,6 +471,8 @@ static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
.height_mm = 64,
.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+ .panel_regs = ltk035c5444t_panel_regs,
+ .panel_regs_len = ARRAY_SIZE(ltk035c5444t_panel_regs),
};
static const struct spi_device_id nv3052c_ids[] = {
--
2.42.0
On 9/11/2023 2:02 AM, John Watts wrote:
> Panel initialization registers are per-display and not tied to the
> controller itself. Different panels will specify their own registers.
> Attach the sequences to the panel info struct so future panels
> can specify their own sequences.
>
> Signed-off-by: John Watts <contact@jookia.org>
> ---
> .../gpu/drm/panel/panel-newvision-nv3052c.c | 25 ++++++++++++-------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> index 307335d0f1fc..b2ad9b3a5eb7 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> @@ -20,11 +20,18 @@
> #include <drm/drm_modes.h>
> #include <drm/drm_panel.h>
>
> +struct nv3052c_reg {
> + u8 cmd;
> + u8 val;
> +};
> +
> struct nv3052c_panel_info {
> const struct drm_display_mode *display_modes;
> unsigned int num_modes;
> u16 width_mm, height_mm;
> u32 bus_format, bus_flags;
> + const struct nv3052c_reg *panel_regs;
> + int panel_regs_len;
> };
>
> struct nv3052c {
> @@ -36,12 +43,7 @@ struct nv3052c {
> struct gpio_desc *reset_gpio;
> };
>
> -struct nv3052c_reg {
> - u8 cmd;
> - u8 val;
> -};
> -
> -static const struct nv3052c_reg nv3052c_panel_regs[] = {
> +static const struct nv3052c_reg ltk035c5444t_panel_regs[] = {
> // EXTC Command set enable, select page 1
> { 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x01 },
> // Mostly unknown registers
> @@ -244,6 +246,7 @@ static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
> static int nv3052c_prepare(struct drm_panel *panel)
> {
> struct nv3052c *priv = to_nv3052c(panel);
> + const struct nv3052c_reg *panel_regs = priv->panel_info->panel_regs;
> struct mipi_dbi *dbi = &priv->dbi;
> unsigned int i;
> int err;
> @@ -260,9 +263,11 @@ static int nv3052c_prepare(struct drm_panel *panel)
> gpiod_set_value_cansleep(priv->reset_gpio, 0);
> msleep(150);
>
> - for (i = 0; i < ARRAY_SIZE(nv3052c_panel_regs); i++) {
> - err = mipi_dbi_command(dbi, nv3052c_panel_regs[i].cmd,
> - nv3052c_panel_regs[i].val);
> + int panel_regs_len = priv->panel_info->panel_regs_len;
Hi John,
Sorry for not catching this earlier -- can you move the declaration of
panel_regs_len to the top of the function? Otherwise this throws a
compiler warning.
With that change,
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
Thanks,
Jessica Zhang
> +
> + for (i = 0; i < panel_regs_len; i++) {
> + err = mipi_dbi_command(dbi, panel_regs[i].cmd,
> + panel_regs[i].val);
>
> if (err) {
> dev_err(priv->dev, "Unable to set register: %d\n", err);
> @@ -466,6 +471,8 @@ static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
> .height_mm = 64,
> .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> + .panel_regs = ltk035c5444t_panel_regs,
> + .panel_regs_len = ARRAY_SIZE(ltk035c5444t_panel_regs),
> };
>
> static const struct spi_device_id nv3052c_ids[] = {
> --
> 2.42.0
>
On 9/11/2023 2:02 AM, John Watts wrote:
> Panel initialization registers are per-display and not tied to the
> controller itself. Different panels will specify their own registers.
> Attach the sequences to the panel info struct so future panels
> can specify their own sequences.
>
> Signed-off-by: John Watts <contact@jookia.org>
> ---
> .../gpu/drm/panel/panel-newvision-nv3052c.c | 25 ++++++++++++-------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> index 307335d0f1fc..b2ad9b3a5eb7 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> @@ -20,11 +20,18 @@
> #include <drm/drm_modes.h>
> #include <drm/drm_panel.h>
>
> +struct nv3052c_reg {
> + u8 cmd;
> + u8 val;
> +};
> +
> struct nv3052c_panel_info {
> const struct drm_display_mode *display_modes;
> unsigned int num_modes;
> u16 width_mm, height_mm;
> u32 bus_format, bus_flags;
> + const struct nv3052c_reg *panel_regs;
> + int panel_regs_len;
Hi John,
Having a separate panel_regs_len field seems a bit unnecessary to me.
Looks like it's only being called in the panel prepare() and I don't
seen any reason why we shouldn't just call the ARRAY_SIZE() macro there.
Thanks,
Jessica Zhang
> };
>
> struct nv3052c {
> @@ -36,12 +43,7 @@ struct nv3052c {
> struct gpio_desc *reset_gpio;
> };
>
> -struct nv3052c_reg {
> - u8 cmd;
> - u8 val;
> -};
> -
> -static const struct nv3052c_reg nv3052c_panel_regs[] = {
> +static const struct nv3052c_reg ltk035c5444t_panel_regs[] = {
> // EXTC Command set enable, select page 1
> { 0xff, 0x30 }, { 0xff, 0x52 }, { 0xff, 0x01 },
> // Mostly unknown registers
> @@ -244,6 +246,7 @@ static inline struct nv3052c *to_nv3052c(struct drm_panel *panel)
> static int nv3052c_prepare(struct drm_panel *panel)
> {
> struct nv3052c *priv = to_nv3052c(panel);
> + const struct nv3052c_reg *panel_regs = priv->panel_info->panel_regs;
> struct mipi_dbi *dbi = &priv->dbi;
> unsigned int i;
> int err;
> @@ -260,9 +263,11 @@ static int nv3052c_prepare(struct drm_panel *panel)
> gpiod_set_value_cansleep(priv->reset_gpio, 0);
> msleep(150);
>
> - for (i = 0; i < ARRAY_SIZE(nv3052c_panel_regs); i++) {
> - err = mipi_dbi_command(dbi, nv3052c_panel_regs[i].cmd,
> - nv3052c_panel_regs[i].val);
> + int panel_regs_len = priv->panel_info->panel_regs_len;
> +
> + for (i = 0; i < panel_regs_len; i++) {
> + err = mipi_dbi_command(dbi, panel_regs[i].cmd,
> + panel_regs[i].val);
>
> if (err) {
> dev_err(priv->dev, "Unable to set register: %d\n", err);
> @@ -466,6 +471,8 @@ static const struct nv3052c_panel_info ltk035c5444t_panel_info = {
> .height_mm = 64,
> .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> + .panel_regs = ltk035c5444t_panel_regs,
> + .panel_regs_len = ARRAY_SIZE(ltk035c5444t_panel_regs),
> };
>
> static const struct spi_device_id nv3052c_ids[] = {
> --
> 2.42.0
>
On Wed, Sep 13, 2023 at 02:34:38PM -0700, Jessica Zhang wrote: > Hi John, > > Having a separate panel_regs_len field seems a bit unnecessary to me. > > Looks like it's only being called in the panel prepare() and I don't seen > any reason why we shouldn't just call the ARRAY_SIZE() macro there. Can you call ARRAY_SIZE on an an array pointer? > > Thanks, > > Jessica Zhang John.
On 9/13/2023 9:09 PM, John Watts wrote: > On Wed, Sep 13, 2023 at 02:34:38PM -0700, Jessica Zhang wrote: >> Hi John, >> >> Having a separate panel_regs_len field seems a bit unnecessary to me. >> >> Looks like it's only being called in the panel prepare() and I don't seen >> any reason why we shouldn't just call the ARRAY_SIZE() macro there. > > Can you call ARRAY_SIZE on an an array pointer? Ah, I'd missed the array pointer declaration. This field is fine then. Thanks, Jessica Zhang > >> >> Thanks, >> >> Jessica Zhang > > John.
© 2016 - 2025 Red Hat, Inc.