* [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification @ 2010-07-26 10:34 Igor Grinberg 2010-07-26 10:34 ` [PATCH 2/2] [ARM] omap: cm-t35: fix video signal on DVI panels Igor Grinberg 2010-07-27 8:00 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration Tomi Valkeinen 0 siblings, 2 replies; 6+ messages in thread From: Igor Grinberg @ 2010-07-26 10:34 UTC (permalink / raw) To: Tomi Valkeinen, Tony Lindgren Cc: linux-omap, linux-fbdev, Igor Grinberg, Mike Rapoport This patch enables platforms to modify the dss device configuration of the generic panel. Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> Signed-off-by: Mike Rapoport <mike@compulab.co.il> --- drivers/video/omap2/displays/panel-generic.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/video/omap2/displays/panel-generic.c b/drivers/video/omap2/displays/panel-generic.c index 300eff5..ad80dd0 100644 --- a/drivers/video/omap2/displays/panel-generic.c +++ b/drivers/video/omap2/displays/panel-generic.c @@ -66,7 +66,7 @@ static void generic_panel_power_off(struct omap_dss_device *dssdev) static int generic_panel_probe(struct omap_dss_device *dssdev) { - dssdev->panel.config = OMAP_DSS_LCD_TFT; + dssdev->panel.config |= OMAP_DSS_LCD_TFT; dssdev->panel.timings = generic_panel_timings; return 0; -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] [ARM] omap: cm-t35: fix video signal on DVI panels 2010-07-26 10:34 [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification Igor Grinberg @ 2010-07-26 10:34 ` Igor Grinberg 2010-07-27 8:00 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration Tomi Valkeinen 1 sibling, 0 replies; 6+ messages in thread From: Igor Grinberg @ 2010-07-26 10:34 UTC (permalink / raw) To: Tomi Valkeinen, Tony Lindgren Cc: linux-omap, linux-fbdev, Igor Grinberg, Mike Rapoport CM-T35 DVI transmitter sampling the data on the raising edge of the pixel clock, therefore the data strobe should happen on the falling edge. Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> Signed-off-by: Mike Rapoport <mike@compulab.co.il> --- arch/arm/mach-omap2/board-cm-t35.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c index e679a2c..1b636f8 100644 --- a/arch/arm/mach-omap2/board-cm-t35.c +++ b/arch/arm/mach-omap2/board-cm-t35.c @@ -384,6 +384,7 @@ static struct omap_dss_device cm_t35_dvi_device = { .driver_name = "generic_panel", .type = OMAP_DISPLAY_TYPE_DPI, .phy.dpi.data_lines = 24, + .panel.config = OMAP_DSS_LCD_IPC | OMAP_DSS_LCD_ONOFF, .platform_enable = cm_t35_panel_enable_dvi, .platform_disable = cm_t35_panel_disable_dvi, }; -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] OMAP: DSS2: enable generic panel configuration 2010-07-26 10:34 [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification Igor Grinberg 2010-07-26 10:34 ` [PATCH 2/2] [ARM] omap: cm-t35: fix video signal on DVI panels Igor Grinberg @ 2010-07-27 8:00 ` Tomi Valkeinen 2010-07-27 13:59 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification Igor Grinberg 1 sibling, 1 reply; 6+ messages in thread From: Tomi Valkeinen @ 2010-07-27 8:00 UTC (permalink / raw) To: ext Igor Grinberg Cc: Tony Lindgren, linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org, Mike Rapoport On Mon, 2010-07-26 at 12:34 +0200, ext Igor Grinberg wrote: > This patch enables platforms to modify the dss device configuration > of the generic panel. > > Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> > Signed-off-by: Mike Rapoport <mike@compulab.co.il> > --- > drivers/video/omap2/displays/panel-generic.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/video/omap2/displays/panel-generic.c b/drivers/video/omap2/displays/panel-generic.c > index 300eff5..ad80dd0 100644 > --- a/drivers/video/omap2/displays/panel-generic.c > +++ b/drivers/video/omap2/displays/panel-generic.c > @@ -66,7 +66,7 @@ static void generic_panel_power_off(struct omap_dss_device *dssdev) > > static int generic_panel_probe(struct omap_dss_device *dssdev) > { > - dssdev->panel.config = OMAP_DSS_LCD_TFT; > + dssdev->panel.config |= OMAP_DSS_LCD_TFT; > dssdev->panel.timings = generic_panel_timings; > > return 0; I don't like this. Panel driver should be the one which decides the config. I think a better solution is to make panel-generic configurable, which has been discussed a bit some time ago on l-o. Briefly: - Add a struct for the configuration variables (a bit like arch/arm/plat-omap/include/plat/nokia-dsi-panel.h). - Fill the struct in the board file, and pass it to the panel driver via omap_dss_device->data pointer. - The panel driver get the struct and uses it to do whatever configuration it needs. I think there are two ways to implement this: 1) Have lots of fields in the struct, including video timings and video signaling information, and the driver uses these directly. 2) Have a panel name in the struct, and the panel driver contains a static list of panels, including configurations for those panels, and the driver selects the configuration based on the panel name given from the board file. (like drivers/video/omap2/displays/panel-taal.c does, except there's currently only one panel defined). While 1. gives perhaps slightly easier configuration, as you just edit the board file, I'd go for 2. because the required configuration is really defined by the panel/chip being used, and so the board file should just state which panel/chip we have, and the driver handles the rest. And 2. makes it also easier to use the same panel/chip on multiple boards. Implementing this would allow us to remove some panel drivers which currently are 99% copies of each other. Tomi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification 2010-07-27 8:00 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration Tomi Valkeinen @ 2010-07-27 13:59 ` Igor Grinberg 2010-08-03 6:45 ` Igor Grinberg 2010-08-03 7:54 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration Tomi Valkeinen 0 siblings, 2 replies; 6+ messages in thread From: Igor Grinberg @ 2010-07-27 13:59 UTC (permalink / raw) To: Tomi Valkeinen Cc: Tony Lindgren, linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org, Mike Rapoport On 07/27/10 11:00, Tomi Valkeinen wrote: > On Mon, 2010-07-26 at 12:34 +0200, ext Igor Grinberg wrote: > >> This patch enables platforms to modify the dss device configuration >> of the generic panel. >> >> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> >> Signed-off-by: Mike Rapoport <mike@compulab.co.il> >> --- >> drivers/video/omap2/displays/panel-generic.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/video/omap2/displays/panel-generic.c b/drivers/video/omap2/displays/panel-generic.c >> index 300eff5..ad80dd0 100644 >> --- a/drivers/video/omap2/displays/panel-generic.c >> +++ b/drivers/video/omap2/displays/panel-generic.c >> @@ -66,7 +66,7 @@ static void generic_panel_power_off(struct omap_dss_device *dssdev) >> >> static int generic_panel_probe(struct omap_dss_device *dssdev) >> { >> - dssdev->panel.config = OMAP_DSS_LCD_TFT; >> + dssdev->panel.config |= OMAP_DSS_LCD_TFT; >> dssdev->panel.timings = generic_panel_timings; >> >> return 0; >> > > I don't like this. Panel driver should be the one which decides the > config. > I agree with this in most cases, but not always. There are certain transmitters (like TI TFP410 dvi transmitter found on cm-t35 and other boards) that can have some of their parameters defined by hardware (strapped), so only the platform knows the correct values for them. One of such parameters is the edge of the clock sampling. > I think a better solution is to make panel-generic configurable, which > has been discussed a bit some time ago on l-o. > No doubt about that :) > Briefly: > > - Add a struct for the configuration variables (a bit like > arch/arm/plat-omap/include/plat/nokia-dsi-panel.h). > > - Fill the struct in the board file, and pass it to the panel driver via > omap_dss_device->data pointer. > > - The panel driver get the struct and uses it to do whatever > configuration it needs. > This is more or less what I've been thinking of, but with slight addition: the panel driver should have a defaults for all the parameters, so there will be no need to provide the whole parameters list, just the ones that are different. > I think there are two ways to implement this: > > 1) Have lots of fields in the struct, including video timings and video > signaling information, and the driver uses these directly. > This seems like a good choice for better flexibility and provides an easy way of dealing with the issues, I've described above. > 2) Have a panel name in the struct, and the panel driver contains a > static list of panels, including configurations for those panels, and > the driver selects the configuration based on the panel name given from > the board file. (like drivers/video/omap2/displays/panel-taal.c does, > except there's currently only one panel defined). > This approach does not deal with the dvi transmitters issue above, unless there will be possibility to define some kind of platform data. Also, if we have a static list of panels with their configurations, there could be panels with (almost) the same parameters defined for a couple of times. > While 1. gives perhaps slightly easier configuration, as you just edit > the board file, I'd go for 2. because the required configuration is > really defined by the panel/chip being used, and so the board file > should just state which panel/chip we have, and the driver handles the > rest. Well, unfortunately, it is not :( > And 2. makes it also easier to use the same panel/chip on multiple > boards. > > Implementing this would allow us to remove some panel drivers which > currently are 99% copies of each other. > > Tomi > > My idea is: generic driver will have built-in defaults, that can (but not necessarily will) be overridden by platform (or other) code on a parameter basis. This will allow other panels (like tdo35s) reuse the generic driver without the need for their special driver. What do you think of it? -- Regards, Igor. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification 2010-07-27 13:59 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification Igor Grinberg @ 2010-08-03 6:45 ` Igor Grinberg 2010-08-03 7:54 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration Tomi Valkeinen 1 sibling, 0 replies; 6+ messages in thread From: Igor Grinberg @ 2010-08-03 6:45 UTC (permalink / raw) To: Tomi Valkeinen Cc: Tony Lindgren, linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org, Mike Rapoport Tomi, Any thoughts? On 07/27/10 16:59, Igor Grinberg wrote: > On 07/27/10 11:00, Tomi Valkeinen wrote: > >> On Mon, 2010-07-26 at 12:34 +0200, ext Igor Grinberg wrote: >> >> >>> This patch enables platforms to modify the dss device configuration >>> of the generic panel. >>> >>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> >>> Signed-off-by: Mike Rapoport <mike@compulab.co.il> >>> --- >>> drivers/video/omap2/displays/panel-generic.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/drivers/video/omap2/displays/panel-generic.c b/drivers/video/omap2/displays/panel-generic.c >>> index 300eff5..ad80dd0 100644 >>> --- a/drivers/video/omap2/displays/panel-generic.c >>> +++ b/drivers/video/omap2/displays/panel-generic.c >>> @@ -66,7 +66,7 @@ static void generic_panel_power_off(struct omap_dss_device *dssdev) >>> >>> static int generic_panel_probe(struct omap_dss_device *dssdev) >>> { >>> - dssdev->panel.config = OMAP_DSS_LCD_TFT; >>> + dssdev->panel.config |= OMAP_DSS_LCD_TFT; >>> dssdev->panel.timings = generic_panel_timings; >>> >>> return 0; >>> >>> >> I don't like this. Panel driver should be the one which decides the >> config. >> >> > I agree with this in most cases, but not always. > There are certain transmitters (like TI TFP410 dvi transmitter found > on cm-t35 and other boards) that can have some of their parameters > defined by hardware (strapped), so only the platform knows the correct > values for them. > One of such parameters is the edge of the clock sampling. > > >> I think a better solution is to make panel-generic configurable, which >> has been discussed a bit some time ago on l-o. >> >> > No doubt about that :) > > >> Briefly: >> >> - Add a struct for the configuration variables (a bit like >> arch/arm/plat-omap/include/plat/nokia-dsi-panel.h). >> >> - Fill the struct in the board file, and pass it to the panel driver via >> omap_dss_device->data pointer. >> >> - The panel driver get the struct and uses it to do whatever >> configuration it needs. >> >> > This is more or less what I've been thinking of, but with slight addition: > the panel driver should have a defaults for all the parameters, > so there will be no need to provide the whole parameters list, > just the ones that are different. > > >> I think there are two ways to implement this: >> >> 1) Have lots of fields in the struct, including video timings and video >> signaling information, and the driver uses these directly. >> >> > This seems like a good choice for better flexibility and > provides an easy way of dealing with the issues, I've described above. > > >> 2) Have a panel name in the struct, and the panel driver contains a >> static list of panels, including configurations for those panels, and >> the driver selects the configuration based on the panel name given from >> the board file. (like drivers/video/omap2/displays/panel-taal.c does, >> except there's currently only one panel defined). >> >> > This approach does not deal with the dvi transmitters issue above, > unless there will be possibility to define some kind of platform data. > Also, if we have a static list of panels with their configurations, > there could be panels with (almost) the same parameters defined > for a couple of times. > > >> While 1. gives perhaps slightly easier configuration, as you just edit >> the board file, I'd go for 2. because the required configuration is >> really defined by the panel/chip being used, and so the board file >> should just state which panel/chip we have, and the driver handles the >> rest. >> > Well, unfortunately, it is not :( > > >> And 2. makes it also easier to use the same panel/chip on multiple >> boards. >> >> Implementing this would allow us to remove some panel drivers which >> currently are 99% copies of each other. >> >> Tomi >> >> >> > My idea is: > generic driver will have built-in defaults, that can > (but not necessarily will) be overridden by platform (or other) code > on a parameter basis. > This will allow other panels (like tdo35s) reuse the generic driver > without the need for their special driver. > > What do you think of it? > > -- Regards, Igor. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] OMAP: DSS2: enable generic panel configuration 2010-07-27 13:59 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification Igor Grinberg 2010-08-03 6:45 ` Igor Grinberg @ 2010-08-03 7:54 ` Tomi Valkeinen 1 sibling, 0 replies; 6+ messages in thread From: Tomi Valkeinen @ 2010-08-03 7:54 UTC (permalink / raw) To: ext Igor Grinberg Cc: Tony Lindgren, linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org, Mike Rapoport On Tue, 2010-07-27 at 15:59 +0200, ext Igor Grinberg wrote: > On 07/27/10 11:00, Tomi Valkeinen wrote: > > On Mon, 2010-07-26 at 12:34 +0200, ext Igor Grinberg wrote: > > > >> This patch enables platforms to modify the dss device configuration > >> of the generic panel. > >> > >> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> > >> Signed-off-by: Mike Rapoport <mike@compulab.co.il> > >> --- > >> drivers/video/omap2/displays/panel-generic.c | 2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> diff --git a/drivers/video/omap2/displays/panel-generic.c b/drivers/video/omap2/displays/panel-generic.c > >> index 300eff5..ad80dd0 100644 > >> --- a/drivers/video/omap2/displays/panel-generic.c > >> +++ b/drivers/video/omap2/displays/panel-generic.c > >> @@ -66,7 +66,7 @@ static void generic_panel_power_off(struct omap_dss_device *dssdev) > >> > >> static int generic_panel_probe(struct omap_dss_device *dssdev) > >> { > >> - dssdev->panel.config = OMAP_DSS_LCD_TFT; > >> + dssdev->panel.config |= OMAP_DSS_LCD_TFT; > >> dssdev->panel.timings = generic_panel_timings; > >> > >> return 0; > >> > > > > I don't like this. Panel driver should be the one which decides the > > config. > > > > I agree with this in most cases, but not always. > There are certain transmitters (like TI TFP410 dvi transmitter found > on cm-t35 and other boards) that can have some of their parameters > defined by hardware (strapped), so only the platform knows the correct > values for them. > One of such parameters is the edge of the clock sampling. Right. But in that case the panel driver should get arguments from the board file, and the panel driver uses those arguments to configure the panel device. > > Briefly: > > > > - Add a struct for the configuration variables (a bit like > > arch/arm/plat-omap/include/plat/nokia-dsi-panel.h). > > > > - Fill the struct in the board file, and pass it to the panel driver via > > omap_dss_device->data pointer. > > > > - The panel driver get the struct and uses it to do whatever > > configuration it needs. > > > > This is more or less what I've been thinking of, but with slight addition: > the panel driver should have a defaults for all the parameters, > so there will be no need to provide the whole parameters list, > just the ones that are different. That's fine, if there's a sane default value for the parameter. > > I think there are two ways to implement this: > > > > 1) Have lots of fields in the struct, including video timings and video > > signaling information, and the driver uses these directly. > > > > This seems like a good choice for better flexibility and > provides an easy way of dealing with the issues, I've described above. > > > 2) Have a panel name in the struct, and the panel driver contains a > > static list of panels, including configurations for those panels, and > > the driver selects the configuration based on the panel name given from > > the board file. (like drivers/video/omap2/displays/panel-taal.c does, > > except there's currently only one panel defined). > > > > This approach does not deal with the dvi transmitters issue above, > unless there will be possibility to define some kind of platform data. If the panel (DVI transmitter) has some non-standard configurations, then it should have a driver of its own. Of course if there's a panel that has just one tiny bit of non-standard functionality (like the DVI transmitter here), it'd be tempting to try to get it use the generic panel and add functionality to generic panel that will allow extra custom configuration. That would perhaps mean some kind of key-value map passed from the board file to panel driver for overriding the panel's default configuration. Depending on the complexity of the implementation, it may or may not be worth it. And how about i2c which can be used to get the resolutions from the DVI display? That would require support in the panel driver, but it doesn't belong in an LCD driver. > Also, if we have a static list of panels with their configurations, > there could be panels with (almost) the same parameters defined > for a couple of times. Sure. But isn't it better to have the panel configs defined only once in the panel driver, than have the same configs defined multiple times in every board file that uses that panel? Also, it's much easier for new boards to use an existing panel: just state the panel's name. The other option would mean browsing through the board files, trying to find some board using the same panel so that you could copy the values. Tomi ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-08-03 7:54 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-07-26 10:34 [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification Igor Grinberg 2010-07-26 10:34 ` [PATCH 2/2] [ARM] omap: cm-t35: fix video signal on DVI panels Igor Grinberg 2010-07-27 8:00 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration Tomi Valkeinen 2010-07-27 13:59 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration modification Igor Grinberg 2010-08-03 6:45 ` Igor Grinberg 2010-08-03 7:54 ` [PATCH 1/2] OMAP: DSS2: enable generic panel configuration Tomi Valkeinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).