| Message ID | 1375220281-11132-4-git-send-email-marex@denx.de |
|---|---|
| State | Accepted |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
On Tue, Jul 30, 2013 at 6:37 PM, Marek Vasut <marex@denx.de> wrote: > Add hook that allow configuring SmartLCD attached the MXS LCDIF > controller operating in System-Mode. This hook can be overriden > by a platform-specific SmartLCD programming routine, which writes > the SmartLCD specific values into it's registers. > > Also, this patch makes sure the SYNC signals are off for the > SmartLCD case. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Anatolij Gustschin <agust@denx.de> > Cc: Fabio Estevam <fabio.estevam@freescale.com> > Cc: Otavio Salvador <otavio@ossystems.com.br> > Cc: Stefano Babic <sbabic@denx.de> > --- > drivers/video/mxsfb.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c > index dbc63a6..78709dd 100644 > --- a/drivers/video/mxsfb.c > +++ b/drivers/video/mxsfb.c > @@ -34,6 +34,17 @@ > > static GraphicDevice panel; > > +/** > + * mxsfb_system_setup() - Fine-tune LCDIF configuration > + * > + * This function is used to adjust the LCDIF configuration. This is usually > + * needed when driving the controller in System-Mode to operate an 8080 or > + * 6800 connected SmartLCD. > + */ > +__weak void mxsfb_system_setup(void) > +{ > +} > + > /* > * DENX M28EVK: > * setenv videomode > @@ -88,6 +99,9 @@ static void mxs_lcd_init(GraphicDevice *panel, > > writel(valid_data << LCDIF_CTRL1_BYTE_PACKING_FORMAT_OFFSET, > ®s->hw_lcdif_ctrl1); > + > + mxsfb_system_setup(); > + > writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | mode->xres, > ®s->hw_lcdif_transfer_count); > > @@ -115,8 +129,10 @@ static void mxs_lcd_init(GraphicDevice *panel, > /* Flush FIFO first */ > writel(LCDIF_CTRL1_FIFO_CLEAR, ®s->hw_lcdif_ctrl1_set); > > +#ifndef CONFIG_VIDEO_MXS_MODE_SYSTEM Mode system or System mode? > /* Sync signals ON */ > setbits_le32(®s->hw_lcdif_vdctrl4, LCDIF_VDCTRL4_SYNC_SIGNALS_ON); > +#endif > > /* FIFO cleared */ > writel(LCDIF_CTRL1_FIFO_CLEAR, ®s->hw_lcdif_ctrl1_clr); > -- > 1.7.10.4 > -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://projetos.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750
Dear Otavio Salvador, > On Tue, Jul 30, 2013 at 6:37 PM, Marek Vasut <marex@denx.de> wrote: > > Add hook that allow configuring SmartLCD attached the MXS LCDIF > > controller operating in System-Mode. This hook can be overriden > > by a platform-specific SmartLCD programming routine, which writes > > the SmartLCD specific values into it's registers. > > > > Also, this patch makes sure the SYNC signals are off for the > > SmartLCD case. > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > Cc: Anatolij Gustschin <agust@denx.de> > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > Cc: Otavio Salvador <otavio@ossystems.com.br> > > Cc: Stefano Babic <sbabic@denx.de> > > --- > > > > drivers/video/mxsfb.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c > > index dbc63a6..78709dd 100644 > > --- a/drivers/video/mxsfb.c > > +++ b/drivers/video/mxsfb.c > > @@ -34,6 +34,17 @@ > > > > static GraphicDevice panel; > > > > +/** > > + * mxsfb_system_setup() - Fine-tune LCDIF configuration > > + * > > + * This function is used to adjust the LCDIF configuration. This is > > usually + * needed when driving the controller in System-Mode to operate > > an 8080 or + * 6800 connected SmartLCD. > > + */ > > +__weak void mxsfb_system_setup(void) > > +{ > > +} > > + > > > > /* > > > > * DENX M28EVK: > > * setenv videomode > > > > @@ -88,6 +99,9 @@ static void mxs_lcd_init(GraphicDevice *panel, > > > > writel(valid_data << LCDIF_CTRL1_BYTE_PACKING_FORMAT_OFFSET, > > > > ®s->hw_lcdif_ctrl1); > > > > + > > + mxsfb_system_setup(); > > + > > > > writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | > > mode->xres, > > > > ®s->hw_lcdif_transfer_count); > > > > @@ -115,8 +129,10 @@ static void mxs_lcd_init(GraphicDevice *panel, > > > > /* Flush FIFO first */ > > writel(LCDIF_CTRL1_FIFO_CLEAR, ®s->hw_lcdif_ctrl1_set); > > > > +#ifndef CONFIG_VIDEO_MXS_MODE_SYSTEM > > Mode system or System mode? This is a name of configuration macro, CONFIG_VIDEO_MXS_MODE_SYSTEM . Otherwise I do not understand what you are asking about. Best regards, Marek Vasut
On Tue, Jul 30, 2013 at 7:45 PM, Marek Vasut <marex@denx.de> wrote: > Dear Otavio Salvador, > >> On Tue, Jul 30, 2013 at 6:37 PM, Marek Vasut <marex@denx.de> wrote: >> > Add hook that allow configuring SmartLCD attached the MXS LCDIF >> > controller operating in System-Mode. This hook can be overriden >> > by a platform-specific SmartLCD programming routine, which writes >> > the SmartLCD specific values into it's registers. >> > >> > Also, this patch makes sure the SYNC signals are off for the >> > SmartLCD case. >> > >> > Signed-off-by: Marek Vasut <marex@denx.de> >> > Cc: Anatolij Gustschin <agust@denx.de> >> > Cc: Fabio Estevam <fabio.estevam@freescale.com> >> > Cc: Otavio Salvador <otavio@ossystems.com.br> >> > Cc: Stefano Babic <sbabic@denx.de> >> > --- >> > >> > drivers/video/mxsfb.c | 16 ++++++++++++++++ >> > 1 file changed, 16 insertions(+) >> > >> > diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c >> > index dbc63a6..78709dd 100644 >> > --- a/drivers/video/mxsfb.c >> > +++ b/drivers/video/mxsfb.c >> > @@ -34,6 +34,17 @@ >> > >> > static GraphicDevice panel; >> > >> > +/** >> > + * mxsfb_system_setup() - Fine-tune LCDIF configuration >> > + * >> > + * This function is used to adjust the LCDIF configuration. This is >> > usually + * needed when driving the controller in System-Mode to operate >> > an 8080 or + * 6800 connected SmartLCD. >> > + */ >> > +__weak void mxsfb_system_setup(void) >> > +{ >> > +} >> > + >> > >> > /* >> > >> > * DENX M28EVK: >> > * setenv videomode >> > >> > @@ -88,6 +99,9 @@ static void mxs_lcd_init(GraphicDevice *panel, >> > >> > writel(valid_data << LCDIF_CTRL1_BYTE_PACKING_FORMAT_OFFSET, >> > >> > ®s->hw_lcdif_ctrl1); >> > >> > + >> > + mxsfb_system_setup(); >> > + >> > >> > writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | >> > mode->xres, >> > >> > ®s->hw_lcdif_transfer_count); >> > >> > @@ -115,8 +129,10 @@ static void mxs_lcd_init(GraphicDevice *panel, >> > >> > /* Flush FIFO first */ >> > writel(LCDIF_CTRL1_FIFO_CLEAR, ®s->hw_lcdif_ctrl1_set); >> > >> > +#ifndef CONFIG_VIDEO_MXS_MODE_SYSTEM >> >> Mode system or System mode? > > This is a name of configuration macro, CONFIG_VIDEO_MXS_MODE_SYSTEM . Otherwise > I do not understand what you are asking about. I got this but accordingly to your commitlog it should be CONFIG_VIDEO_MXS_SYSTEM_MODE to match the description. Any reason to not use this name? -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://projetos.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750
Dear Otavio Salvador, > On Tue, Jul 30, 2013 at 7:45 PM, Marek Vasut <marex@denx.de> wrote: > > Dear Otavio Salvador, > > > >> On Tue, Jul 30, 2013 at 6:37 PM, Marek Vasut <marex@denx.de> wrote: > >> > Add hook that allow configuring SmartLCD attached the MXS LCDIF > >> > controller operating in System-Mode. This hook can be overriden > >> > by a platform-specific SmartLCD programming routine, which writes > >> > the SmartLCD specific values into it's registers. > >> > > >> > Also, this patch makes sure the SYNC signals are off for the > >> > SmartLCD case. > >> > > >> > Signed-off-by: Marek Vasut <marex@denx.de> > >> > Cc: Anatolij Gustschin <agust@denx.de> > >> > Cc: Fabio Estevam <fabio.estevam@freescale.com> > >> > Cc: Otavio Salvador <otavio@ossystems.com.br> > >> > Cc: Stefano Babic <sbabic@denx.de> > >> > --- > >> > > >> > drivers/video/mxsfb.c | 16 ++++++++++++++++ > >> > 1 file changed, 16 insertions(+) > >> > > >> > diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c > >> > index dbc63a6..78709dd 100644 > >> > --- a/drivers/video/mxsfb.c > >> > +++ b/drivers/video/mxsfb.c > >> > @@ -34,6 +34,17 @@ > >> > > >> > static GraphicDevice panel; > >> > > >> > +/** > >> > + * mxsfb_system_setup() - Fine-tune LCDIF configuration > >> > + * > >> > + * This function is used to adjust the LCDIF configuration. This is > >> > usually + * needed when driving the controller in System-Mode to > >> > operate an 8080 or + * 6800 connected SmartLCD. > >> > + */ > >> > +__weak void mxsfb_system_setup(void) > >> > +{ > >> > +} > >> > + > >> > > >> > /* > >> > > >> > * DENX M28EVK: > >> > * setenv videomode > >> > > >> > @@ -88,6 +99,9 @@ static void mxs_lcd_init(GraphicDevice *panel, > >> > > >> > writel(valid_data << LCDIF_CTRL1_BYTE_PACKING_FORMAT_OFFSET, > >> > > >> > ®s->hw_lcdif_ctrl1); > >> > > >> > + > >> > + mxsfb_system_setup(); > >> > + > >> > > >> > writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | > >> > mode->xres, > >> > > >> > ®s->hw_lcdif_transfer_count); > >> > > >> > @@ -115,8 +129,10 @@ static void mxs_lcd_init(GraphicDevice *panel, > >> > > >> > /* Flush FIFO first */ > >> > writel(LCDIF_CTRL1_FIFO_CLEAR, ®s->hw_lcdif_ctrl1_set); > >> > > >> > +#ifndef CONFIG_VIDEO_MXS_MODE_SYSTEM > >> > >> Mode system or System mode? > > > > This is a name of configuration macro, CONFIG_VIDEO_MXS_MODE_SYSTEM . > > Otherwise I do not understand what you are asking about. > > I got this but accordingly to your commitlog it should be > CONFIG_VIDEO_MXS_SYSTEM_MODE to match the description. Any reason to > not use this name? Sorry, is this name of the macro mentioned in the commit log anywhere? I don't see it there. I believe grepping for CONFIG_VIDEO_MXS_MODE will make it easier in the future -- if we were to support more modes -- to find all occurances of such conditionals. Best regards, Marek Vasut
Hi Marek, On 30/07/2013 23:37, Marek Vasut wrote: > Add hook that allow configuring SmartLCD attached the MXS LCDIF > controller operating in System-Mode. This hook can be overriden > by a platform-specific SmartLCD programming routine, which writes > the SmartLCD specific values into it's registers. > > Also, this patch makes sure the SYNC signals are off for the > SmartLCD case. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Anatolij Gustschin <agust@denx.de> > Cc: Fabio Estevam <fabio.estevam@freescale.com> > Cc: Otavio Salvador <otavio@ossystems.com.br> > Cc: Stefano Babic <sbabic@denx.de> > --- > drivers/video/mxsfb.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c > index dbc63a6..78709dd 100644 > --- a/drivers/video/mxsfb.c > +++ b/drivers/video/mxsfb.c > @@ -34,6 +34,17 @@ > > static GraphicDevice panel; > > +/** > + * mxsfb_system_setup() - Fine-tune LCDIF configuration > + * > + * This function is used to adjust the LCDIF configuration. This is usually > + * needed when driving the controller in System-Mode to operate an 8080 or > + * 6800 connected SmartLCD. > + */ > +__weak void mxsfb_system_setup(void) > +{ > +} > + We have no easy way to know if a function is declared weak, but generally a lot of weak functions are board specific. Try to use the same naming schema, and rename this one as board_mxfb_setup() (or something like that). Or better: why cannot we use board_video_init(), as this name is already used by other SOCs ? > /* > * DENX M28EVK: > * setenv videomode > @@ -88,6 +99,9 @@ static void mxs_lcd_init(GraphicDevice *panel, > > writel(valid_data << LCDIF_CTRL1_BYTE_PACKING_FORMAT_OFFSET, > ®s->hw_lcdif_ctrl1); > + > + mxsfb_system_setup(); > + > writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | mode->xres, > ®s->hw_lcdif_transfer_count); > > @@ -115,8 +129,10 @@ static void mxs_lcd_init(GraphicDevice *panel, > /* Flush FIFO first */ > writel(LCDIF_CTRL1_FIFO_CLEAR, ®s->hw_lcdif_ctrl1_set); > > +#ifndef CONFIG_VIDEO_MXS_MODE_SYSTEM > /* Sync signals ON */ > setbits_le32(®s->hw_lcdif_vdctrl4, LCDIF_VDCTRL4_SYNC_SIGNALS_ON); > +#endif Question: is there anything wrong to move it into mxsfb_system_setup() ? I would prefer to avoid adding a new not documented CONFIG_ only to set a bit... Best regards, Stefano Babic
Dear Stefano Babic, > Hi Marek, > > On 30/07/2013 23:37, Marek Vasut wrote: > > Add hook that allow configuring SmartLCD attached the MXS LCDIF > > controller operating in System-Mode. This hook can be overriden > > by a platform-specific SmartLCD programming routine, which writes > > the SmartLCD specific values into it's registers. > > > > Also, this patch makes sure the SYNC signals are off for the > > SmartLCD case. > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > Cc: Anatolij Gustschin <agust@denx.de> > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > Cc: Otavio Salvador <otavio@ossystems.com.br> > > Cc: Stefano Babic <sbabic@denx.de> > > --- > > > > drivers/video/mxsfb.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c > > index dbc63a6..78709dd 100644 > > --- a/drivers/video/mxsfb.c > > +++ b/drivers/video/mxsfb.c > > @@ -34,6 +34,17 @@ > > > > static GraphicDevice panel; > > > > +/** > > + * mxsfb_system_setup() - Fine-tune LCDIF configuration > > + * > > + * This function is used to adjust the LCDIF configuration. This is > > usually + * needed when driving the controller in System-Mode to operate > > an 8080 or + * 6800 connected SmartLCD. > > + */ > > +__weak void mxsfb_system_setup(void) > > +{ > > +} > > + > > We have no easy way to know if a function is declared weak, but > generally a lot of weak functions are board specific. > > Try to use the same naming schema, and rename this one as > board_mxfb_setup() (or something like that). > > Or better: why cannot we use board_video_init(), as this name is already > used by other SOCs ? This is system-mode specific and needs to be called at this particular place. > > /* > > > > * DENX M28EVK: > > * setenv videomode > > > > @@ -88,6 +99,9 @@ static void mxs_lcd_init(GraphicDevice *panel, > > > > writel(valid_data << LCDIF_CTRL1_BYTE_PACKING_FORMAT_OFFSET, > > > > ®s->hw_lcdif_ctrl1); > > > > + > > + mxsfb_system_setup(); > > + > > > > writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | > > mode->xres, > > > > ®s->hw_lcdif_transfer_count); > > > > @@ -115,8 +129,10 @@ static void mxs_lcd_init(GraphicDevice *panel, > > > > /* Flush FIFO first */ > > writel(LCDIF_CTRL1_FIFO_CLEAR, ®s->hw_lcdif_ctrl1_set); > > > > +#ifndef CONFIG_VIDEO_MXS_MODE_SYSTEM > > > > /* Sync signals ON */ > > setbits_le32(®s->hw_lcdif_vdctrl4, LCDIF_VDCTRL4_SYNC_SIGNALS_ON); > > > > +#endif > > Question: is there anything wrong to move it into mxsfb_system_setup() ? > I would prefer to avoid adding a new not documented CONFIG_ only to set > a bit... Yes, the order in which the LCD is configured must be preserved as-is. Best regards, Marek Vasut
Hi Marek, On 31/07/2013 15:22, Marek Vasut wrote: >> >> Question: is there anything wrong to move it into mxsfb_system_setup() ? >> I would prefer to avoid adding a new not documented CONFIG_ only to set >> a bit... > > Yes, the order in which the LCD is configured must be preserved as-is. > Ok, understood - sometimes is not possible to suimplify. Best regards, Stefano Babic
Dear Stefano Babic, > Hi Marek, > > On 31/07/2013 15:22, Marek Vasut wrote: > >> Question: is there anything wrong to move it into mxsfb_system_setup() ? > >> I would prefer to avoid adding a new not documented CONFIG_ only to set > >> a bit... > > > > Yes, the order in which the LCD is configured must be preserved as-is. > > Ok, understood - sometimes is not possible to suimplify. I believe SmartLCDs should be considered a crime against hardware design (and transitively an offense against software engineers' human rights). Just my 5 cents ... Best regards, Marek Vasut
On Tue, 30 Jul 2013 23:37:53 +0200 Marek Vasut <marex@denx.de> wrote: > Add hook that allow configuring SmartLCD attached the MXS LCDIF > controller operating in System-Mode. This hook can be overriden > by a platform-specific SmartLCD programming routine, which writes > the SmartLCD specific values into it's registers. > > Also, this patch makes sure the SYNC signals are off for the > SmartLCD case. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Anatolij Gustschin <agust@denx.de> > Cc: Fabio Estevam <fabio.estevam@freescale.com> > Cc: Otavio Salvador <otavio@ossystems.com.br> > Cc: Stefano Babic <sbabic@denx.de> > --- > drivers/video/mxsfb.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) Applied to u-boot-video/master. Thanks! Anatolij
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c index dbc63a6..78709dd 100644 --- a/drivers/video/mxsfb.c +++ b/drivers/video/mxsfb.c @@ -34,6 +34,17 @@ static GraphicDevice panel; +/** + * mxsfb_system_setup() - Fine-tune LCDIF configuration + * + * This function is used to adjust the LCDIF configuration. This is usually + * needed when driving the controller in System-Mode to operate an 8080 or + * 6800 connected SmartLCD. + */ +__weak void mxsfb_system_setup(void) +{ +} + /* * DENX M28EVK: * setenv videomode @@ -88,6 +99,9 @@ static void mxs_lcd_init(GraphicDevice *panel, writel(valid_data << LCDIF_CTRL1_BYTE_PACKING_FORMAT_OFFSET, ®s->hw_lcdif_ctrl1); + + mxsfb_system_setup(); + writel((mode->yres << LCDIF_TRANSFER_COUNT_V_COUNT_OFFSET) | mode->xres, ®s->hw_lcdif_transfer_count); @@ -115,8 +129,10 @@ static void mxs_lcd_init(GraphicDevice *panel, /* Flush FIFO first */ writel(LCDIF_CTRL1_FIFO_CLEAR, ®s->hw_lcdif_ctrl1_set); +#ifndef CONFIG_VIDEO_MXS_MODE_SYSTEM /* Sync signals ON */ setbits_le32(®s->hw_lcdif_vdctrl4, LCDIF_VDCTRL4_SYNC_SIGNALS_ON); +#endif /* FIFO cleared */ writel(LCDIF_CTRL1_FIFO_CLEAR, ®s->hw_lcdif_ctrl1_clr);
Add hook that allow configuring SmartLCD attached the MXS LCDIF controller operating in System-Mode. This hook can be overriden by a platform-specific SmartLCD programming routine, which writes the SmartLCD specific values into it's registers. Also, this patch makes sure the SYNC signals are off for the SmartLCD case. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Anatolij Gustschin <agust@denx.de> Cc: Fabio Estevam <fabio.estevam@freescale.com> Cc: Otavio Salvador <otavio@ossystems.com.br> Cc: Stefano Babic <sbabic@denx.de> --- drivers/video/mxsfb.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)