[PATCH 1/4] OMAP: DSS2: Clean up a switch-case

[PATCH 1/4] OMAP: DSS2: Clean up a switch-case

[Tomi Valkeinen]

Support for the display interface was checked in a separate switch-case.
There's no reason for that, and this patch handles the fail code path in
the same switch-case where the display initialization is done.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/display.c |   27 +++------------------------
 1 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
index 22dd7a4..3f4fa0b 100644
--- a/drivers/video/omap2/dss/display.c
+++ b/drivers/video/omap2/dss/display.c
@@ -396,29 +396,6 @@ void dss_init_device(struct platform_device *pdev,
 	switch (dssdev->type) {
 #ifdef CONFIG_OMAP2_DSS_DPI
 	case OMAP_DISPLAY_TYPE_DPI:
-#endif
-#ifdef CONFIG_OMAP2_DSS_RFBI
-	case OMAP_DISPLAY_TYPE_DBI:
-#endif
-#ifdef CONFIG_OMAP2_DSS_SDI
-	case OMAP_DISPLAY_TYPE_SDI:
-#endif
-#ifdef CONFIG_OMAP2_DSS_DSI
-	case OMAP_DISPLAY_TYPE_DSI:
-#endif
-#ifdef CONFIG_OMAP2_DSS_VENC
-	case OMAP_DISPLAY_TYPE_VENC:
-#endif
-		break;
-	default:
-		DSSERR("Support for display '%s' not compiled in.\n",
-				dssdev->name);
-		return;
-	}
-
-	switch (dssdev->type) {
-#ifdef CONFIG_OMAP2_DSS_DPI
-	case OMAP_DISPLAY_TYPE_DPI:
 		r = dpi_init_display(dssdev);
 		break;
 #endif
@@ -443,7 +420,9 @@ void dss_init_device(struct platform_device *pdev,
 		break;
 #endif
 	default:
-		BUG();
+		DSSERR("Support for display '%s' not compiled in.\n",
+				dssdev->name);
+		return;
 	}
 
 	if (r) {
-- 
1.7.1

[PATCH 2/4] OMAP: DSS2: FEATURES: Remove SDI from 3630 displays

[Tomi Valkeinen]

OMAP 3630 does not support SDI. Split omap3_dss_supported_displays into
3430 and 3630 entries, and remove the SDI from 3630 entry.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dss_features.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index cf3ef69..f4367f6 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -73,7 +73,7 @@ static const enum omap_display_type omap2_dss_supported_displays[] = {
 	OMAP_DISPLAY_TYPE_VENC,
 };
 
-static const enum omap_display_type omap3_dss_supported_displays[] = {
+static const enum omap_display_type omap3430_dss_supported_displays[] = {
 	/* OMAP_DSS_CHANNEL_LCD */
 	OMAP_DISPLAY_TYPE_DPI | OMAP_DISPLAY_TYPE_DBI |
 	OMAP_DISPLAY_TYPE_SDI | OMAP_DISPLAY_TYPE_DSI,
@@ -82,6 +82,15 @@ static const enum omap_display_type omap3_dss_supported_displays[] = {
 	OMAP_DISPLAY_TYPE_VENC,
 };
 
+static const enum omap_display_type omap3630_dss_supported_displays[] = {
+	/* OMAP_DSS_CHANNEL_LCD */
+	OMAP_DISPLAY_TYPE_DPI | OMAP_DISPLAY_TYPE_DBI |
+	OMAP_DISPLAY_TYPE_DSI,
+
+	/* OMAP_DSS_CHANNEL_DIGIT */
+	OMAP_DISPLAY_TYPE_VENC,
+};
+
 static const enum omap_display_type omap4_dss_supported_displays[] = {
 	/* OMAP_DSS_CHANNEL_LCD */
 	OMAP_DISPLAY_TYPE_DBI | OMAP_DISPLAY_TYPE_DSI,
@@ -161,7 +170,7 @@ static struct omap_dss_features omap3430_dss_features = {
 
 	.num_mgrs = 2,
 	.num_ovls = 3,
-	.supported_displays = omap3_dss_supported_displays,
+	.supported_displays = omap3430_dss_supported_displays,
 	.supported_color_modes = omap3_dss_supported_color_modes,
 };
 
@@ -176,7 +185,7 @@ static struct omap_dss_features omap3630_dss_features = {
 
 	.num_mgrs = 2,
 	.num_ovls = 3,
-	.supported_displays = omap3_dss_supported_displays,
+	.supported_displays = omap3630_dss_supported_displays,
 	.supported_color_modes = omap3_dss_supported_color_modes,
 };
 
-- 
1.7.1

[PATCH 3/4] OMAP: DSS2: FEATURES: Remove DSI & SDI from OMAP2

[Tomi Valkeinen]

OMAP2 doesn't support SDI or DSI. Remove them from
omap2_dss_supported_displays.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dss_features.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index f4367f6..fe22d11 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -66,8 +66,7 @@ static const struct dss_reg_field omap3_dss_reg_fields[] = {
 
 static const enum omap_display_type omap2_dss_supported_displays[] = {
 	/* OMAP_DSS_CHANNEL_LCD */
-	OMAP_DISPLAY_TYPE_DPI | OMAP_DISPLAY_TYPE_DBI |
-	OMAP_DISPLAY_TYPE_SDI | OMAP_DISPLAY_TYPE_DSI,
+	OMAP_DISPLAY_TYPE_DPI | OMAP_DISPLAY_TYPE_DBI,
 
 	/* OMAP_DSS_CHANNEL_DIGIT */
 	OMAP_DISPLAY_TYPE_VENC,
-- 
1.7.1

[PATCH 4/4] OMAP: DSS2: Check for SDI HW before accessing SDI registers

[Tomi Valkeinen]

Only OMAP 3430 hardware has SDI support. The availability of SDI HW can
be found out by checking if the LCD channel supports SDI displays.

This patch checks for SDI HW support before accessing SDI registers,
which fixes a crash on OMAP4 when SDI SW support is compiled in.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/display.c |   10 ++++++++++
 drivers/video/omap2/dss/dss.c     |   29 ++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
index 3f4fa0b..58459f4 100644
--- a/drivers/video/omap2/dss/display.c
+++ b/drivers/video/omap2/dss/display.c
@@ -30,6 +30,7 @@
 
 #include <plat/display.h>
 #include "dss.h"
+#include "dss_features.h"
 
 static LIST_HEAD(display_list);
 
@@ -392,6 +393,15 @@ void dss_init_device(struct platform_device *pdev,
 	struct device_attribute *attr;
 	int i;
 	int r;
+	enum omap_display_type supported;
+
+	supported = dss_feat_get_supported_displays(dssdev->channel);
+
+	if (!(supported & dssdev->type)) {
+		DSSERR("Unsupported display interface for display '%s'.\n",
+				dssdev->name);
+		return;
+	}
 
 	switch (dssdev->type) {
 #ifdef CONFIG_OMAP2_DSS_DPI
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 5a93e66..b3e9dd1 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -32,6 +32,7 @@
 #include <plat/display.h>
 #include <plat/clock.h>
 #include "dss.h"
+#include "dss_features.h"
 
 #define DSS_SZ_REGS			SZ_512
 
@@ -110,10 +111,11 @@ void dss_save_context(void)
 	SR(SYSCONFIG);
 	SR(CONTROL);
 
-#ifdef CONFIG_OMAP2_DSS_SDI
-	SR(SDI_CONTROL);
-	SR(PLL_CONTROL);
-#endif
+	if (dss_feat_get_supported_displays(OMAP_DSS_CHANNEL_LCD) &
+			OMAP_DISPLAY_TYPE_SDI) {
+		SR(SDI_CONTROL);
+		SR(PLL_CONTROL);
+	}
 }
 
 void dss_restore_context(void)
@@ -124,10 +126,11 @@ void dss_restore_context(void)
 	RR(SYSCONFIG);
 	RR(CONTROL);
 
-#ifdef CONFIG_OMAP2_DSS_SDI
-	RR(SDI_CONTROL);
-	RR(PLL_CONTROL);
-#endif
+	if (dss_feat_get_supported_displays(OMAP_DSS_CHANNEL_LCD) &
+			OMAP_DISPLAY_TYPE_SDI) {
+		RR(SDI_CONTROL);
+		RR(PLL_CONTROL);
+	}
 }
 
 #undef SR
@@ -259,9 +262,13 @@ void dss_dump_regs(struct seq_file *s)
 	DUMPREG(DSS_SYSSTATUS);
 	DUMPREG(DSS_IRQSTATUS);
 	DUMPREG(DSS_CONTROL);
-	DUMPREG(DSS_SDI_CONTROL);
-	DUMPREG(DSS_PLL_CONTROL);
-	DUMPREG(DSS_SDI_STATUS);
+
+	if (dss_feat_get_supported_displays(OMAP_DSS_CHANNEL_LCD) &
+			OMAP_DISPLAY_TYPE_SDI) {
+		DUMPREG(DSS_SDI_CONTROL);
+		DUMPREG(DSS_PLL_CONTROL);
+		DUMPREG(DSS_SDI_STATUS);
+	}
 
 	dss_clk_disable(DSS_CLK_ICK | DSS_CLK_FCK);
 #undef DUMPREG
-- 
1.7.1

Re: [PATCH 3/4] OMAP: DSS2: FEATURES: Remove DSI & SDI from OMAP2

[archit taneja]

Hi,

On Thursday 24 February 2011 07:04 PM, Valkeinen, Tomi wrote:
> OMAP2 doesn't support SDI or DSI. Remove them from
> omap2_dss_supported_displays.
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/dss_features.c |    3 +--
>   1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
> index f4367f6..fe22d11 100644
> --- a/drivers/video/omap2/dss/dss_features.c
> +++ b/drivers/video/omap2/dss/dss_features.c
> @@ -66,8 +66,7 @@ static const struct dss_reg_field omap3_dss_reg_fields[] = {
>
>   static const enum omap_display_type omap2_dss_supported_displays[] = {
>   	/* OMAP_DSS_CHANNEL_LCD */
> -	OMAP_DISPLAY_TYPE_DPI | OMAP_DISPLAY_TYPE_DBI |
> -	OMAP_DISPLAY_TYPE_SDI | OMAP_DISPLAY_TYPE_DSI,
> +	OMAP_DISPLAY_TYPE_DPI | OMAP_DISPLAY_TYPE_DBI,

That's a blunder from my end! Thanks for the fix.

Archit

Re: [PATCH 4/4] OMAP: DSS2: Check for SDI HW before accessing SDI registers

[archit taneja]

Hi,

On Thursday 24 February 2011 07:04 PM, Valkeinen, Tomi wrote:
> Only OMAP 3430 hardware has SDI support. The availability of SDI HW can
> be found out by checking if the LCD channel supports SDI displays.
>
> This patch checks for SDI HW support before accessing SDI registers,
> which fixes a crash on OMAP4 when SDI SW support is compiled in.
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/display.c |   10 ++++++++++
>   drivers/video/omap2/dss/dss.c     |   29 ++++++++++++++++++-----------
>   2 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
> index 3f4fa0b..58459f4 100644
> --- a/drivers/video/omap2/dss/display.c
> +++ b/drivers/video/omap2/dss/display.c
> @@ -30,6 +30,7 @@
>
>   #include<plat/display.h>
>   #include "dss.h"
> +#include "dss_features.h"
>
>   static LIST_HEAD(display_list);
>
> @@ -392,6 +393,15 @@ void dss_init_device(struct platform_device *pdev,
>   	struct device_attribute *attr;
>   	int i;
>   	int r;
> +	enum omap_display_type supported;
> +
> +	supported = dss_feat_get_supported_displays(dssdev->channel);
> +
> +	if (!(supported&  dssdev->type)) {
> +		DSSERR("Unsupported display interface for display '%s'.\n",
> +				dssdev->name);
> +		return;
> +	}

This would make it necessary to specify the channel in the board file, 
especially digit. I think this patch should also add the channel 
parameters for all board files which add a tv display.

<snip>

Archit

Re: [PATCH 4/4] OMAP: DSS2: Check for SDI HW before accessing SDI registers

[Tomi Valkeinen]

On Tue, 2011-03-01 at 00:49 -0600, Taneja, Archit wrote:
> Hi,
> 
> On Thursday 24 February 2011 07:04 PM, Valkeinen, Tomi wrote:
> > Only OMAP 3430 hardware has SDI support. The availability of SDI HW can
> > be found out by checking if the LCD channel supports SDI displays.
> >
> > This patch checks for SDI HW support before accessing SDI registers,
> > which fixes a crash on OMAP4 when SDI SW support is compiled in.
> >
> > Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> > ---
> >   drivers/video/omap2/dss/display.c |   10 ++++++++++
> >   drivers/video/omap2/dss/dss.c     |   29 ++++++++++++++++++-----------
> >   2 files changed, 28 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
> > index 3f4fa0b..58459f4 100644
> > --- a/drivers/video/omap2/dss/display.c
> > +++ b/drivers/video/omap2/dss/display.c
> > @@ -30,6 +30,7 @@
> >
> >   #include<plat/display.h>
> >   #include "dss.h"
> > +#include "dss_features.h"
> >
> >   static LIST_HEAD(display_list);
> >
> > @@ -392,6 +393,15 @@ void dss_init_device(struct platform_device *pdev,
> >   	struct device_attribute *attr;
> >   	int i;
> >   	int r;
> > +	enum omap_display_type supported;
> > +
> > +	supported = dss_feat_get_supported_displays(dssdev->channel);
> > +
> > +	if (!(supported&  dssdev->type)) {
> > +		DSSERR("Unsupported display interface for display '%s'.\n",
> > +				dssdev->name);
> > +		return;
> > +	}
> 
> This would make it necessary to specify the channel in the board file, 
> especially digit. I think this patch should also add the channel 
> parameters for all board files which add a tv display.

Argh. You're right, dssdev->channel is not what I thought it is. That is
rather confusing. I remember this was discussed when that
dssdev->channel was introduced.

I believe this should work if I change dssdev->channel to
dssdev->manager->id.

But this dssdev->channel has to be fixed somehow, I'm 100% sure this
won't be the last time somebody tries to use it =).

 Tomi

Re: [PATCH 4/4] OMAP: DSS2: Check for SDI HW before accessing SDI registers

[Tomi Valkeinen]

On Tue, 2011-03-01 at 00:49 -0600, Taneja, Archit wrote:
> Hi,
> 
> On Thursday 24 February 2011 07:04 PM, Valkeinen, Tomi wrote:
> > Only OMAP 3430 hardware has SDI support. The availability of SDI HW can
> > be found out by checking if the LCD channel supports SDI displays.
> >
> > This patch checks for SDI HW support before accessing SDI registers,
> > which fixes a crash on OMAP4 when SDI SW support is compiled in.
> >
> > Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> > ---
> >   drivers/video/omap2/dss/display.c |   10 ++++++++++
> >   drivers/video/omap2/dss/dss.c     |   29 ++++++++++++++++++-----------
> >   2 files changed, 28 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
> > index 3f4fa0b..58459f4 100644
> > --- a/drivers/video/omap2/dss/display.c
> > +++ b/drivers/video/omap2/dss/display.c
> > @@ -30,6 +30,7 @@
> >
> >   #include<plat/display.h>
> >   #include "dss.h"
> > +#include "dss_features.h"
> >
> >   static LIST_HEAD(display_list);
> >
> > @@ -392,6 +393,15 @@ void dss_init_device(struct platform_device *pdev,
> >   	struct device_attribute *attr;
> >   	int i;
> >   	int r;
> > +	enum omap_display_type supported;
> > +
> > +	supported = dss_feat_get_supported_displays(dssdev->channel);
> > +
> > +	if (!(supported&  dssdev->type)) {
> > +		DSSERR("Unsupported display interface for display '%s'.\n",
> > +				dssdev->name);
> > +		return;
> > +	}
> 
> This would make it necessary to specify the channel in the board file, 
> especially digit. I think this patch should also add the channel 
> parameters for all board files which add a tv display.

Also, I don't even see why this piece of code is in that patch... The
patch is about checking for SDI HW.

 Tomi

Re: [PATCH 4/4] OMAP: DSS2: Check for SDI HW before accessing SDI registers

[archit taneja]

Hi,

On Tuesday 01 March 2011 01:08 PM, Valkeinen, Tomi wrote:
> On Tue, 2011-03-01 at 00:49 -0600, Taneja, Archit wrote:
>> Hi,
>>
>> On Thursday 24 February 2011 07:04 PM, Valkeinen, Tomi wrote:
>>> Only OMAP 3430 hardware has SDI support. The availability of SDI HW can
>>> be found out by checking if the LCD channel supports SDI displays.
>>>
>>> This patch checks for SDI HW support before accessing SDI registers,
>>> which fixes a crash on OMAP4 when SDI SW support is compiled in.
>>>
>>> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
>>> ---
>>>    drivers/video/omap2/dss/display.c |   10 ++++++++++
>>>    drivers/video/omap2/dss/dss.c     |   29 ++++++++++++++++++-----------
>>>    2 files changed, 28 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
>>> index 3f4fa0b..58459f4 100644
>>> --- a/drivers/video/omap2/dss/display.c
>>> +++ b/drivers/video/omap2/dss/display.c
>>> @@ -30,6 +30,7 @@
>>>
>>>    #include<plat/display.h>
>>>    #include "dss.h"
>>> +#include "dss_features.h"
>>>
>>>    static LIST_HEAD(display_list);
>>>
>>> @@ -392,6 +393,15 @@ void dss_init_device(struct platform_device *pdev,
>>>    	struct device_attribute *attr;
>>>    	int i;
>>>    	int r;
>>> +	enum omap_display_type supported;
>>> +
>>> +	supported = dss_feat_get_supported_displays(dssdev->channel);
>>> +
>>> +	if (!(supported&   dssdev->type)) {
>>> +		DSSERR("Unsupported display interface for display '%s'.\n",
>>> +				dssdev->name);
>>> +		return;
>>> +	}
>>
>> This would make it necessary to specify the channel in the board file,
>> especially digit. I think this patch should also add the channel
>> parameters for all board files which add a tv display.
>
> Argh. You're right, dssdev->channel is not what I thought it is. That is
> rather confusing. I remember this was discussed when that
> dssdev->channel was introduced.
>
> I believe this should work if I change dssdev->channel to
> dssdev->manager->id.
>

I am not sure it will work, dss_init_device is called before 
dss_recheck_connections which sets the manager for the display.

Hence, dssdev->manager will be NULL.

> But this dssdev->channel has to be fixed somehow, I'm 100% sure this
> won't be the last time somebody tries to use it =).
>

I think after the managers are set, we should somehow make the channel 
parameter "void".

Archit

Re: [PATCH 4/4] OMAP: DSS2: Check for SDI HW before accessing SDI registers

[Tomi Valkeinen]

On Tue, 2011-03-01 at 01:46 -0600, Taneja, Archit wrote:
> Hi,
> 
> On Tuesday 01 March 2011 01:08 PM, Valkeinen, Tomi wrote:
> > On Tue, 2011-03-01 at 00:49 -0600, Taneja, Archit wrote:
> >> Hi,
> >>
> >> On Thursday 24 February 2011 07:04 PM, Valkeinen, Tomi wrote:
> >>> Only OMAP 3430 hardware has SDI support. The availability of SDI HW can
> >>> be found out by checking if the LCD channel supports SDI displays.
> >>>
> >>> This patch checks for SDI HW support before accessing SDI registers,
> >>> which fixes a crash on OMAP4 when SDI SW support is compiled in.
> >>>
> >>> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> >>> ---
> >>>    drivers/video/omap2/dss/display.c |   10 ++++++++++
> >>>    drivers/video/omap2/dss/dss.c     |   29 ++++++++++++++++++-----------
> >>>    2 files changed, 28 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
> >>> index 3f4fa0b..58459f4 100644
> >>> --- a/drivers/video/omap2/dss/display.c
> >>> +++ b/drivers/video/omap2/dss/display.c
> >>> @@ -30,6 +30,7 @@
> >>>
> >>>    #include<plat/display.h>
> >>>    #include "dss.h"
> >>> +#include "dss_features.h"
> >>>
> >>>    static LIST_HEAD(display_list);
> >>>
> >>> @@ -392,6 +393,15 @@ void dss_init_device(struct platform_device *pdev,
> >>>    	struct device_attribute *attr;
> >>>    	int i;
> >>>    	int r;
> >>> +	enum omap_display_type supported;
> >>> +
> >>> +	supported = dss_feat_get_supported_displays(dssdev->channel);
> >>> +
> >>> +	if (!(supported&   dssdev->type)) {
> >>> +		DSSERR("Unsupported display interface for display '%s'.\n",
> >>> +				dssdev->name);
> >>> +		return;
> >>> +	}
> >>
> >> This would make it necessary to specify the channel in the board file,
> >> especially digit. I think this patch should also add the channel
> >> parameters for all board files which add a tv display.
> >
> > Argh. You're right, dssdev->channel is not what I thought it is. That is
> > rather confusing. I remember this was discussed when that
> > dssdev->channel was introduced.
> >
> > I believe this should work if I change dssdev->channel to
> > dssdev->manager->id.
> >
> 
> I am not sure it will work, dss_init_device is called before 
> dss_recheck_connections which sets the manager for the display.
> 
> Hence, dssdev->manager will be NULL.

True.

> > But this dssdev->channel has to be fixed somehow, I'm 100% sure this
> > won't be the last time somebody tries to use it =).
> >
> 
> I think after the managers are set, we should somehow make the channel 
> parameter "void".

Perhaps not. It is a valid piece of information, isn't it? It tells to
which output channel this device is connected on the hardware level.
That and display type is needed to describe how the panel is connected
and how the DSS outputs should be set up.

Looking at manager.c:omap_dss_set_device, which checks if a device can
be attached to a manager, it only checks:

	if ((mgr->supported_displays & dssdev->type) == 0) {

Which means it doesn't work properly for OMAP4. Here we would also need
the dssdev->channel information, wouldn't we?

But this seems a bit confusing to me. I need to study this more.

For the time being, I think I can just remove the piece of code above,
as 1) it doesn't work right and 2) it's only needed in case the board
file is set up wrong.

 Tomi

Re: [PATCH 4/4] OMAP: DSS2: Check for SDI HW before accessing SDI registers

[archit taneja]

On Tuesday 01 March 2011 01:30 PM, Valkeinen, Tomi wrote:
> On Tue, 2011-03-01 at 01:46 -0600, Taneja, Archit wrote:
>> Hi,
>>
>> On Tuesday 01 March 2011 01:08 PM, Valkeinen, Tomi wrote:
>>> On Tue, 2011-03-01 at 00:49 -0600, Taneja, Archit wrote:
>>>> Hi,
>>>>
>>>> On Thursday 24 February 2011 07:04 PM, Valkeinen, Tomi wrote:
>>>>> Only OMAP 3430 hardware has SDI support. The availability of SDI HW can
>>>>> be found out by checking if the LCD channel supports SDI displays.
>>>>>
>>>>> This patch checks for SDI HW support before accessing SDI registers,
>>>>> which fixes a crash on OMAP4 when SDI SW support is compiled in.
>>>>>
>>>>> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
>>>>> ---
>>>>>     drivers/video/omap2/dss/display.c |   10 ++++++++++
>>>>>     drivers/video/omap2/dss/dss.c     |   29 ++++++++++++++++++-----------
>>>>>     2 files changed, 28 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
>>>>> index 3f4fa0b..58459f4 100644
>>>>> --- a/drivers/video/omap2/dss/display.c
>>>>> +++ b/drivers/video/omap2/dss/display.c
>>>>> @@ -30,6 +30,7 @@
>>>>>
>>>>>     #include<plat/display.h>
>>>>>     #include "dss.h"
>>>>> +#include "dss_features.h"
>>>>>
>>>>>     static LIST_HEAD(display_list);
>>>>>
>>>>> @@ -392,6 +393,15 @@ void dss_init_device(struct platform_device *pdev,
>>>>>     	struct device_attribute *attr;
>>>>>     	int i;
>>>>>     	int r;
>>>>> +	enum omap_display_type supported;
>>>>> +
>>>>> +	supported = dss_feat_get_supported_displays(dssdev->channel);
>>>>> +
>>>>> +	if (!(supported&    dssdev->type)) {
>>>>> +		DSSERR("Unsupported display interface for display '%s'.\n",
>>>>> +				dssdev->name);
>>>>> +		return;
>>>>> +	}
>>>>
>>>> This would make it necessary to specify the channel in the board file,
>>>> especially digit. I think this patch should also add the channel
>>>> parameters for all board files which add a tv display.
>>>
>>> Argh. You're right, dssdev->channel is not what I thought it is. That is
>>> rather confusing. I remember this was discussed when that
>>> dssdev->channel was introduced.
>>>
>>> I believe this should work if I change dssdev->channel to
>>> dssdev->manager->id.
>>>
>>
>> I am not sure it will work, dss_init_device is called before
>> dss_recheck_connections which sets the manager for the display.
>>
>> Hence, dssdev->manager will be NULL.
>
> True.
>
>>> But this dssdev->channel has to be fixed somehow, I'm 100% sure this
>>> won't be the last time somebody tries to use it =).
>>>
>>
>> I think after the managers are set, we should somehow make the channel
>> parameter "void".
>
> Perhaps not. It is a valid piece of information, isn't it? It tells to
> which output channel this device is connected on the hardware level.
> That and display type is needed to describe how the panel is connected
> and how the DSS outputs should be set up.
>
> Looking at manager.c:omap_dss_set_device, which checks if a device can
> be attached to a manager, it only checks:
>
> 	if ((mgr->supported_displays&  dssdev->type) == 0) {
>
> Which means it doesn't work properly for OMAP4. Here we would also need
> the dssdev->channel information, wouldn't we?

We have already done the check with dssdev->channel before calling 
omap_dss_set_device in dss_recheck_connections in overlay.c, but since 
dss_recheck_connections is a one time affair, and set_device / 
unset_device can be called later on, I agree that it would be a better 
idea to move the dssdev->channel checks into omap_dss_set_device rather 
than keeping it recheck_connections.

>
> But this seems a bit confusing to me. I need to study this more.
>
> For the time being, I think I can just remove the piece of code above,
> as 1) it doesn't work right and 2) it's only needed in case the board
> file is set up wrong.
>

Okay.