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

[archit taneja <archit@ti.com>]

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