Re: [PATCH 1/3] OMAP: use generic panel data in board files
On Mon, 2010-11-08 at 20:43 +0100, ext Bryan Wu wrote: > On Mon, Nov 8, 2010 at 7:26 AM, Tomi Valkeinen > wrote: > > Hi, > > > > On Fri, 2010-11-05 at 20:43 +0100, ext Bryan Wu wrote: > >> static struct omap_dss_device sdp3430_dvi_device = { > >> .name = "dvi", > >> - .driver_name= "generic_panel", > >> - .type = OMAP_DISPLAY_TYPE_DPI, > >> - .phy.dpi.data_lines = 24, > > > > Why do you remove type and datalines configuration? You do this for the > > other panels also. > > > > I found this is common in all the generic dpi panel, but the value > depends on the specific panel. I move this to the panel configuration > data and will set this value in panel-dpi driver. Do you think need I > keep them here? Actually I found all of them are 24 for generic one. > 16 or 18 for others. Both type and .phy.dpi.data_lines tell about the connection to the panel, not about the panel itself. Thus I think it's more consistent to have them defined in the board file. Also, it's really up to the board HW design how many lines are connected from OMAP to the panel. You could connect only 16, for example, so it's something that has to be defined in the board file. Tomi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] OMAP: use generic panel data in board files
On Mon, Nov 8, 2010 at 7:19 AM, Tomi Valkeinen wrote: > Hi, > > On Fri, 2010-11-05 at 22:17 +0100, ext Taneja, Archit wrote: >> Hi, >> >> linux-omap-ow...@vger.kernel.org wrote: >> > Introduce struct panel config data in panel.h, which will be >> > used to match the right panel configurations in generic DPI >> > panel driver and other future dsi panel drivers. >> > >> > Still keep sharp_ls_panel, since the sharp_ls_panel driver >> > contains blacklight control driver code which will be moved >> > out later. Then we can use generic DPI driver for sharp_ls_panel. >> > >> > Signed-off-by: Bryan Wu >> > --- >> > arch/arm/mach-omap2/board-3430sdp.c | 10 +++- >> > arch/arm/mach-omap2/board-am3517evm.c | 19 +-- >> > arch/arm/mach-omap2/board-cm-t35.c | 19 +-- >> > arch/arm/mach-omap2/board-devkit8000.c | 22 +--- >> > arch/arm/mach-omap2/board-igep0020.c | 10 +++- >> > arch/arm/mach-omap2/board-omap3beagle.c | 10 +++- >> > arch/arm/mach-omap2/board-omap3evm.c | 10 +++- >> > arch/arm/mach-omap2/board-omap3stalker.c | 19 +-- >> > arch/arm/plat-omap/include/plat/nokia-dsi-panel.h | 31 --- >> > arch/arm/plat-omap/include/plat/panel.h | 57 >> > + drivers/video/omap2/displays/panel-taal.c | >> > 26 -- 11 files changed, 148 insertions(+), 85 deletions(-) delete >> > mode 100644 arch/arm/plat-omap/include/plat/nokia-dsi-panel.h >> > create mode 100644 arch/arm/plat-omap/include/plat/panel.h >> >> I am not totally sure about the need of removal of nokia-dsi-panel.h >> and the addition of a generic panel.h. >> >> I guess the reason why nokia-dsi-panel.h was introduced (and others that >> will be introduced in future) was to easily represent panel-specific data >> across different boards that use the same panel. > > Right. Don't touch panel-taal.c or nokia-dsi-panel.h, they are not > related to this DPI panel stuff. > >> For example, if there is a new panel which for some reson uses 2 pins, one >> for switching off and one for switching on the panel, then it would make >> sense >> to introduce a structure for this panel having members on_gpio and off_gpio, >> this >> struct could then be passed and accessed through dssdev->data in the panel's >> probe >> giving us the option to have different gpio numbers for different boards but >> finally >> being accessed in the same way by the driver. >> >> So, there isn't a need to generalize this struct and the corresponding >> header file >> for all panels and make it available for all board files. >> >> As far as the dummy panels are concerned, since the "name" is the only >> criteria to >> differentiate the panel, I think passing the name to the data member of >> omap_dss_device >> should itself be enough for the generic dpi driver to handle things. > > I think it's a bit confusing to just put a string to the void *data > member, but fortunately we don't need to ponder about that: the generic > DPI panel should be told about reset_gpio, max_backlight_level, > platform_enable/disable and set/get_backlight, which need to be passed > in a struct. So a header and a struct is needed for this generic DPI > driver. > Yeah, so we just put a name field in a struct such as generic-dpi-panel-data in generic-dpi-panel.h And our driver file will be panel-generic-dpi.c? Thanks, -- Bryan Wu Kernel Developer +86.138-1617-6545 Mobile Ubuntu Kernel Team Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] OMAP: use generic panel data in board files
On Mon, Nov 8, 2010 at 7:26 AM, Tomi Valkeinen wrote: > Hi, > > On Fri, 2010-11-05 at 20:43 +0100, ext Bryan Wu wrote: >> Introduce struct panel config data in panel.h, which will be used to match >> the right panel configurations in generic DPI panel driver and other future >> dsi panel drivers. >> >> Still keep sharp_ls_panel, since the sharp_ls_panel driver contains >> blacklight >> control driver code which will be moved out later. Then we can use generic >> DPI >> driver for sharp_ls_panel. > > As mentioned in the other mail, don't touch panel-taal or > nokia-dsi-panel.h. They are not related to this change. > Got it. I just found nokia-dsi-panel.h is some kind of confusing and tried to unify them. Obviously I should not touch them in this patchset. I will remove them in next version. > The panel.h file should be spesific for the generic panel driver, so > name the .c and .h files similarly. (yes, panel-taal.c and > nokia-dsi-panel.h are not good examples for this, but I have a patch > fixing it, I just haven't had time to push it forward =). > OK, I got it. Actually I did that, but wanna unify the nokia-dis-panel.h somehow. I will change back. > And remember that the kernel should compile and work after each > individual patch in the patch set. In this patch you set the boards to > use dvi_panel driver, but there is no dvi_panel driver yet. > Got it. I will fold some patches together. > Also some comments inline. > >> Signed-off-by: Bryan Wu >> --- >> arch/arm/mach-omap2/board-3430sdp.c | 10 +++- >> arch/arm/mach-omap2/board-am3517evm.c | 19 +-- >> arch/arm/mach-omap2/board-cm-t35.c | 19 +-- >> arch/arm/mach-omap2/board-devkit8000.c | 22 +--- >> arch/arm/mach-omap2/board-igep0020.c | 10 +++- >> arch/arm/mach-omap2/board-omap3beagle.c | 10 +++- >> arch/arm/mach-omap2/board-omap3evm.c | 10 +++- >> arch/arm/mach-omap2/board-omap3stalker.c | 19 +-- >> arch/arm/plat-omap/include/plat/nokia-dsi-panel.h | 31 --- >> arch/arm/plat-omap/include/plat/panel.h | 57 >> + >> drivers/video/omap2/displays/panel-taal.c | 26 -- >> 11 files changed, 148 insertions(+), 85 deletions(-) >> delete mode 100644 arch/arm/plat-omap/include/plat/nokia-dsi-panel.h >> create mode 100644 arch/arm/plat-omap/include/plat/panel.h >> >> diff --git a/arch/arm/mach-omap2/board-3430sdp.c >> b/arch/arm/mach-omap2/board-3430sdp.c >> index 4e3742c..859b4e5 100644 >> --- a/arch/arm/mach-omap2/board-3430sdp.c >> +++ b/arch/arm/mach-omap2/board-3430sdp.c >> @@ -38,6 +38,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> >> @@ -270,11 +271,14 @@ static struct omap_dss_device sdp3430_lcd_device = { >> .platform_disable = sdp3430_panel_disable_lcd, >> }; >> >> +static struct panel_data dvi_panel = { >> + .name = "generic", >> +}; >> + >> static struct omap_dss_device sdp3430_dvi_device = { >> .name = "dvi", >> - .driver_name = "generic_panel", >> - .type = OMAP_DISPLAY_TYPE_DPI, >> - .phy.dpi.data_lines = 24, > > Why do you remove type and datalines configuration? You do this for the > other panels also. > I found this is common in all the generic dpi panel, but the value depends on the specific panel. I move this to the panel configuration data and will set this value in panel-dpi driver. Do you think need I keep them here? Actually I found all of them are 24 for generic one. 16 or 18 for others. -- Bryan Wu Kernel Developer +86.138-1617-6545 Mobile Ubuntu Kernel Team Canonical Ltd. www.canonical.com Ubuntu - Linux for human beings | www.ubuntu.com -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] OMAP: use generic panel data in board files
Hi, On Fri, 2010-11-05 at 20:43 +0100, ext Bryan Wu wrote: > Introduce struct panel config data in panel.h, which will be used to match > the right panel configurations in generic DPI panel driver and other future > dsi panel drivers. > > Still keep sharp_ls_panel, since the sharp_ls_panel driver contains blacklight > control driver code which will be moved out later. Then we can use generic DPI > driver for sharp_ls_panel. As mentioned in the other mail, don't touch panel-taal or nokia-dsi-panel.h. They are not related to this change. The panel.h file should be spesific for the generic panel driver, so name the .c and .h files similarly. (yes, panel-taal.c and nokia-dsi-panel.h are not good examples for this, but I have a patch fixing it, I just haven't had time to push it forward =). And remember that the kernel should compile and work after each individual patch in the patch set. In this patch you set the boards to use dvi_panel driver, but there is no dvi_panel driver yet. Also some comments inline. > Signed-off-by: Bryan Wu > --- > arch/arm/mach-omap2/board-3430sdp.c | 10 +++- > arch/arm/mach-omap2/board-am3517evm.c | 19 +-- > arch/arm/mach-omap2/board-cm-t35.c| 19 +-- > arch/arm/mach-omap2/board-devkit8000.c| 22 +--- > arch/arm/mach-omap2/board-igep0020.c | 10 +++- > arch/arm/mach-omap2/board-omap3beagle.c | 10 +++- > arch/arm/mach-omap2/board-omap3evm.c | 10 +++- > arch/arm/mach-omap2/board-omap3stalker.c | 19 +-- > arch/arm/plat-omap/include/plat/nokia-dsi-panel.h | 31 --- > arch/arm/plat-omap/include/plat/panel.h | 57 > + > drivers/video/omap2/displays/panel-taal.c | 26 -- > 11 files changed, 148 insertions(+), 85 deletions(-) > delete mode 100644 arch/arm/plat-omap/include/plat/nokia-dsi-panel.h > create mode 100644 arch/arm/plat-omap/include/plat/panel.h > > diff --git a/arch/arm/mach-omap2/board-3430sdp.c > b/arch/arm/mach-omap2/board-3430sdp.c > index 4e3742c..859b4e5 100644 > --- a/arch/arm/mach-omap2/board-3430sdp.c > +++ b/arch/arm/mach-omap2/board-3430sdp.c > @@ -38,6 +38,7 @@ > #include > #include > #include > +#include > > #include > > @@ -270,11 +271,14 @@ static struct omap_dss_device sdp3430_lcd_device = { > .platform_disable = sdp3430_panel_disable_lcd, > }; > > +static struct panel_data dvi_panel = { > + .name = "generic", > +}; > + > static struct omap_dss_device sdp3430_dvi_device = { > .name = "dvi", > - .driver_name= "generic_panel", > - .type = OMAP_DISPLAY_TYPE_DPI, > - .phy.dpi.data_lines = 24, Why do you remove type and datalines configuration? You do this for the other panels also. Tomi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3] OMAP: use generic panel data in board files
Hi, On Fri, 2010-11-05 at 22:17 +0100, ext Taneja, Archit wrote: > Hi, > > linux-omap-ow...@vger.kernel.org wrote: > > Introduce struct panel config data in panel.h, which will be > > used to match the right panel configurations in generic DPI > > panel driver and other future dsi panel drivers. > > > > Still keep sharp_ls_panel, since the sharp_ls_panel driver > > contains blacklight control driver code which will be moved > > out later. Then we can use generic DPI driver for sharp_ls_panel. > > > > Signed-off-by: Bryan Wu > > --- > > arch/arm/mach-omap2/board-3430sdp.c | 10 +++- > > arch/arm/mach-omap2/board-am3517evm.c | 19 +-- > > arch/arm/mach-omap2/board-cm-t35.c| 19 +-- > > arch/arm/mach-omap2/board-devkit8000.c| 22 +--- > > arch/arm/mach-omap2/board-igep0020.c | 10 +++- > > arch/arm/mach-omap2/board-omap3beagle.c | 10 +++- > > arch/arm/mach-omap2/board-omap3evm.c | 10 +++- > > arch/arm/mach-omap2/board-omap3stalker.c | 19 +-- > > arch/arm/plat-omap/include/plat/nokia-dsi-panel.h | 31 --- > > arch/arm/plat-omap/include/plat/panel.h | 57 > > + drivers/video/omap2/displays/panel-taal.c | > > 26 -- 11 files changed, 148 insertions(+), 85 deletions(-) delete > > mode 100644 arch/arm/plat-omap/include/plat/nokia-dsi-panel.h > > create mode 100644 arch/arm/plat-omap/include/plat/panel.h > > I am not totally sure about the need of removal of nokia-dsi-panel.h > and the addition of a generic panel.h. > > I guess the reason why nokia-dsi-panel.h was introduced (and others that > will be introduced in future) was to easily represent panel-specific data > across different boards that use the same panel. Right. Don't touch panel-taal.c or nokia-dsi-panel.h, they are not related to this DPI panel stuff. > For example, if there is a new panel which for some reson uses 2 pins, one > for switching off and one for switching on the panel, then it would make sense > to introduce a structure for this panel having members on_gpio and off_gpio, > this > struct could then be passed and accessed through dssdev->data in the panel's > probe > giving us the option to have different gpio numbers for different boards but > finally > being accessed in the same way by the driver. > > So, there isn't a need to generalize this struct and the corresponding header > file > for all panels and make it available for all board files. > > As far as the dummy panels are concerned, since the "name" is the only > criteria to > differentiate the panel, I think passing the name to the data member of > omap_dss_device > should itself be enough for the generic dpi driver to handle things. I think it's a bit confusing to just put a string to the void *data member, but fortunately we don't need to ponder about that: the generic DPI panel should be told about reset_gpio, max_backlight_level, platform_enable/disable and set/get_backlight, which need to be passed in a struct. So a header and a struct is needed for this generic DPI driver. Tomi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3] OMAP: use generic panel data in board files
Hi, linux-omap-ow...@vger.kernel.org wrote: > Introduce struct panel config data in panel.h, which will be > used to match the right panel configurations in generic DPI > panel driver and other future dsi panel drivers. > > Still keep sharp_ls_panel, since the sharp_ls_panel driver > contains blacklight control driver code which will be moved > out later. Then we can use generic DPI driver for sharp_ls_panel. > > Signed-off-by: Bryan Wu > --- > arch/arm/mach-omap2/board-3430sdp.c | 10 +++- > arch/arm/mach-omap2/board-am3517evm.c | 19 +-- > arch/arm/mach-omap2/board-cm-t35.c| 19 +-- > arch/arm/mach-omap2/board-devkit8000.c| 22 +--- > arch/arm/mach-omap2/board-igep0020.c | 10 +++- > arch/arm/mach-omap2/board-omap3beagle.c | 10 +++- > arch/arm/mach-omap2/board-omap3evm.c | 10 +++- > arch/arm/mach-omap2/board-omap3stalker.c | 19 +-- > arch/arm/plat-omap/include/plat/nokia-dsi-panel.h | 31 --- > arch/arm/plat-omap/include/plat/panel.h | 57 > + drivers/video/omap2/displays/panel-taal.c | > 26 -- 11 files changed, 148 insertions(+), 85 deletions(-) delete > mode 100644 arch/arm/plat-omap/include/plat/nokia-dsi-panel.h > create mode 100644 arch/arm/plat-omap/include/plat/panel.h I am not totally sure about the need of removal of nokia-dsi-panel.h and the addition of a generic panel.h. I guess the reason why nokia-dsi-panel.h was introduced (and others that will be introduced in future) was to easily represent panel-specific data across different boards that use the same panel. For example, if there is a new panel which for some reson uses 2 pins, one for switching off and one for switching on the panel, then it would make sense to introduce a structure for this panel having members on_gpio and off_gpio, this struct could then be passed and accessed through dssdev->data in the panel's probe giving us the option to have different gpio numbers for different boards but finally being accessed in the same way by the driver. So, there isn't a need to generalize this struct and the corresponding header file for all panels and make it available for all board files. As far as the dummy panels are concerned, since the "name" is the only criteria to differentiate the panel, I think passing the name to the data member of omap_dss_device should itself be enough for the generic dpi driver to handle things. > > diff --git a/arch/arm/mach-omap2/board-3430sdp.c > b/arch/arm/mach-omap2/board-3430sdp.c > index 4e3742c..859b4e5 100644 > --- a/arch/arm/mach-omap2/board-3430sdp.c > +++ b/arch/arm/mach-omap2/board-3430sdp.c > @@ -38,6 +38,7 @@ > #include > #include > #include > +#include > > #include > > @@ -270,11 +271,14 @@ static struct omap_dss_device sdp3430_lcd_device = { > .platform_disable = sdp3430_panel_disable_lcd, > }; > > +static struct panel_data dvi_panel = { > + .name = "generic", > +}; > + > static struct omap_dss_device sdp3430_dvi_device = { .name > = "dvi", > - .driver_name= "generic_panel", > - .type = OMAP_DISPLAY_TYPE_DPI, > - .phy.dpi.data_lines = 24, > + .driver_name= "dpi_panel", > + .data = &dvi_panel, > .platform_enable= sdp3430_panel_enable_dvi, > .platform_disable = sdp3430_panel_disable_dvi, > }; > diff --git a/arch/arm/mach-omap2/board-am3517evm.c > b/arch/arm/mach-omap2/board-am3517evm.c > index 0739950..9b2b6ff 100644 > --- a/arch/arm/mach-omap2/board-am3517evm.c > +++ b/arch/arm/mach-omap2/board-am3517evm.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > > #include "mux.h" > #include "control.h" > @@ -303,11 +304,14 @@ static void > am3517_evm_panel_disable_lcd(struct omap_dss_device *dssdev) > lcd_enabled = > 0; } > > +static struct panel_data lcd_panel = { > + .name = "sharp_lq", > +}; > + > static struct omap_dss_device am3517_evm_lcd_device = { > - .type = OMAP_DISPLAY_TYPE_DPI, > .name = "lcd", > - .driver_name= "sharp_lq_panel", > - .phy.dpi.data_lines = 16, > + .driver_name= "dpi_panel", > + .data = &lcd_panel, > .platform_enable= am3517_evm_panel_enable_lcd, > .platform_disable = am3517_evm_panel_disable_lcd, }; > @@ -346,11 +350,14 @@ static void > am3517_evm_panel_disable_dvi(struct omap_dss_device *dssdev) > dvi_enabled = > 0; } > > +static struct panel_data dvi_panel = { > + .name = "generic", > +}; > + > static struct omap_dss_device am3517_evm_dvi_device = { > - .type = OMAP_DISPLAY_TYPE_DPI, > .name = "dvi", > - .driver_name= "gener