DSS2 patch series

DSS2 patch series

[Taneja, Archit]

Hi,

We had pushed three patch series while you were out. We received
some good comments on them. We wanted to know if you can point out
more fixes and susuggestions before we rework them.

https://patchwork.kernel.org/patch/111901/

https://patchwork.kernel.org/patch/112648/

https://patchwork.kernel.org/patch/112653/

Thanks,

Archit

Re: DSS2 patch series

[Tomi Valkeinen]

Hi,

On Tue, 2010-07-27 at 10:27 +0200, ext Taneja, Archit wrote:
> Hi,
> 
> We had pushed three patch series while you were out. We received
> some good comments on them. We wanted to know if you can point out
> more fixes and susuggestions before we rework them.
> 
> https://patchwork.kernel.org/patch/111901/
> 
> https://patchwork.kernel.org/patch/112648/
> 
> https://patchwork.kernel.org/patch/112653/

Do you have updated patches for these? It would be easier to review new
ones than the old ones.

 Tomi

RE: DSS2 patch series

[Taneja, Archit]

Hi,

> On Tue, 2010-07-27 at 10:27 +0200, ext Taneja, Archit wrote:
> > Hi,
> > 
> > We had pushed three patch series while you were out. We 
> received some 
> > good comments on them. We wanted to know if you can point out more 
> > fixes and susuggestions before we rework them.
> > 
> > https://patchwork.kernel.org/patch/111901/
> > 
> > https://patchwork.kernel.org/patch/112648/
> > 
> > https://patchwork.kernel.org/patch/112653/
> 
> Do you have updated patches for these? It would be easier to 
> review new ones than the old ones.
> 
>  Tomi

The comments on the existing patches have been mostly related
to minimizing the number of patches in each series, moving some
#defines to header files and one or two typos.

I haven't reworked these patches yet, I wanted to know if you
could review the changes you had suggested on the v1 of the series
below:

(https://patchwork.kernel.org/patch/111901/)

If you want I can rework them and send again. No hassles.

Thanks,

Archit

> 
> 
> 

RE: DSS2 patch series

[Tomi Valkeinen]

On Mon, 2010-08-02 at 14:05 +0200, ext Taneja, Archit wrote:
> Hi,
> 
> > On Tue, 2010-07-27 at 10:27 +0200, ext Taneja, Archit wrote:
> > > Hi,
> > > 
> > > We had pushed three patch series while you were out. We 
> > received some 
> > > good comments on them. We wanted to know if you can point out more 
> > > fixes and susuggestions before we rework them.
> > > 
> > > https://patchwork.kernel.org/patch/111901/
> > > 
> > > https://patchwork.kernel.org/patch/112648/
> > > 
> > > https://patchwork.kernel.org/patch/112653/
> > 
> > Do you have updated patches for these? It would be easier to 
> > review new ones than the old ones.
> > 
> >  Tomi
> 
> The comments on the existing patches have been mostly related
> to minimizing the number of patches in each series, moving some
> #defines to header files and one or two typos.
> 
> I haven't reworked these patches yet, I wanted to know if you
> could review the changes you had suggested on the v1 of the series
> below:

I think the register handling looks much better now. Although defines
like this:

+#define DISPC_DEFAULT_COLOR(ch)	DISPC_REG(ch == OMAP_DSS_CHANNEL_LCD \
+				? (0x004C) : (ch == OMAP_DSS_CHANNEL_DIGIT ? \
+				(0x0050) : (0x03AC)))

makes me wonder if a separate lookup table would be cleaner. But perhaps
it's not too bad yet.

Also, we should think how to reduce if (cpu_is_omap44xx()) lines. There
should be some kind of DSS capability list somewhere, which would tell
the features available. I haven't thought this more, but it'd be very
nice if we could use the DSS HW version number to decide what features
there are.

However, TI answered that information about DSS HW version numbers is
not available, and thus cannot be used =(. Perhaps you could try to dig
out some information from inside TI?

And, as others have commented, consider how to split the patches. The
most important rule is that every stage in the patch set has to compile.

One thing that I personally think makes patch sets clearer, is to divide
a new functionality into two parts: 1) modify the old code, without
adding new functionality, so that the new functionality is easier to add
2) add the new functionality.

As an example, if we look at the new DISPC registers patch:

There could first be a patch that introduces DISPC_XXX(channel) style
registers (even if there's only one channel available) and modifies the
functions accordingly. 

The second patch would introduce OMAP4 registers and whatever new
functionality these registers require.

This way it would be easy to study and test the first patch, as it
shouldn't introduce any new functionality, but just "shuffles the code
around".

And it would be easy to study the second patch, as the needed base
functionality is already there, and there's lot less new stuff added.

 Tomi

RE: DSS2 patch series

[Taneja, Archit]

Hi,

> On Mon, 2010-08-02 at 14:05 +0200, ext Taneja, Archit wrote:
> > Hi,
> > 
> > > On Tue, 2010-07-27 at 10:27 +0200, ext Taneja, Archit wrote:
> > > > Hi,
> > > > 
> > > > We had pushed three patch series while you were out. We
> > > received some
> > > > good comments on them. We wanted to know if you can 
> point out more 
> > > > fixes and susuggestions before we rework them.
> > > > 
> > > > https://patchwork.kernel.org/patch/111901/
> > > > 
> > > > https://patchwork.kernel.org/patch/112648/
> > > > 
> > > > https://patchwork.kernel.org/patch/112653/
> > > 
> > > Do you have updated patches for these? It would be easier 
> to review 
> > > new ones than the old ones.
> > > 
> > >  Tomi
> > 
> > The comments on the existing patches have been mostly related to 
> > minimizing the number of patches in each series, moving 
> some #defines 
> > to header files and one or two typos.
> > 
> > I haven't reworked these patches yet, I wanted to know if you could 
> > review the changes you had suggested on the v1 of the series
> > below:
> 
> I think the register handling looks much better now. Although 
> defines like this:
> 
> +#define DISPC_DEFAULT_COLOR(ch)	DISPC_REG(ch == 
> OMAP_DSS_CHANNEL_LCD \
> +				? (0x004C) : (ch == 
> OMAP_DSS_CHANNEL_DIGIT ? \
> +				(0x0050) : (0x03AC)))
> 
> makes me wonder if a separate lookup table would be cleaner. 
> But perhaps it's not too bad yet.
> 
> Also, we should think how to reduce if (cpu_is_omap44xx()) 
> lines. There should be some kind of DSS capability list 
> somewhere, which would tell the features available. I haven't 
> thought this more, but it'd be very nice if we could use the 
> DSS HW version number to decide what features there are.
> 
> However, TI answered that information about DSS HW version 
> numbers is not available, and thus cannot be used =(. Perhaps 
> you could try to dig out some information from inside TI?

I will try to find out more about the version numbers, I was wondering
if the DSS_REVISION, DISPC_REVISION etc registers could be enough to
differentiate between DSS hardware features. We could put a revision
struct in the corresponding global structures.

> 
> And, as others have commented, consider how to split the 
> patches. The most important rule is that every stage in the 
> patch set has to compile.
> 
> One thing that I personally think makes patch sets clearer, 
> is to divide a new functionality into two parts: 1) modify 
> the old code, without adding new functionality, so that the 
> new functionality is easier to add
> 2) add the new functionality.
> 
> As an example, if we look at the new DISPC registers patch:
> 
> There could first be a patch that introduces 
> DISPC_XXX(channel) style registers (even if there's only one 
> channel available) and modifies the functions accordingly. 
> 
> The second patch would introduce OMAP4 registers and whatever 
> new functionality these registers require.
> 
> This way it would be easy to study and test the first patch, 
> as it shouldn't introduce any new functionality, but just 
> "shuffles the code around".
> 
> And it would be easy to study the second patch, as the needed 
> base functionality is already there, and there's lot less new 
> stuff added.
> 
>  Tomi

Yes, this makes quite a lot of sense, I will adopt this approach
from now onwards.

Archit

RE: DSS2 patch series

[Taneja, Archit]

Hi,

> 
> Also, we should think how to reduce if (cpu_is_omap44xx()) 
> lines. There should be some kind of DSS capability list 
> somewhere, which would tell the features available. I haven't 
> thought this more, but it'd be very nice if we could use the 
> DSS HW version number to decide what features there are.
> 
> However, TI answered that information about DSS HW version 
> numbers is not available, and thus cannot be used =(. Perhaps 
> you could try to dig out some information from inside TI?
> 

I read the DSS_REVISON, DISPC_REVISION etc registers on 3430, 3630, 4430:

3430: DSS rev 2.0, DISPC rev 3.0, RFBI rev 1.0, DSI rev 1.0, VENC rev 2
3630: DSS rev 2.0, DISPC rev 3.0, RFBI rev 1.0, DSI rev 1.0, VENC rev 2
4430: DSS rev 4.0, DISPC rev 4.0, DSI rev 3.0, RFBI rev 3.5

I haven't tried on OMAP2 yet..

Don't you think these revision numbers are enough to differentiate the
features of each IP block?

For example, if there are changes in the bit field lengths of video pipeline
Registers from omap2 to 3, we can use the dispc revision number to decide to
include the feature instead of having a cpu_is_omap24xx() change? Same for
global alpha, new color modes and so on.

Similarly, if a new video pipeline is introduced in OMAP4, we can use the DISPC
revision number to decide the number of pipelines. We could use DSS Revision numbers
to introduce changes like new interrupt lines, clocks and addition or removal of IP
blocks.

Do you mean something else by DSS HW version numbers? Would it be different/more precise
compared to the read from the DSS_REVISON register?

Thanks,

Archit

RE: DSS2 patch series

[Tomi Valkeinen]

On Thu, 2010-08-05 at 09:06 +0200, ext Taneja, Archit wrote:
> Hi,
> 
> > 
> > Also, we should think how to reduce if (cpu_is_omap44xx()) 
> > lines. There should be some kind of DSS capability list 
> > somewhere, which would tell the features available. I haven't 
> > thought this more, but it'd be very nice if we could use the 
> > DSS HW version number to decide what features there are.
> > 
> > However, TI answered that information about DSS HW version 
> > numbers is not available, and thus cannot be used =(. Perhaps 
> > you could try to dig out some information from inside TI?
> > 
> 
> I read the DSS_REVISON, DISPC_REVISION etc registers on 3430, 3630, 4430:
> 
> 3430: DSS rev 2.0, DISPC rev 3.0, RFBI rev 1.0, DSI rev 1.0, VENC rev 2
> 3630: DSS rev 2.0, DISPC rev 3.0, RFBI rev 1.0, DSI rev 1.0, VENC rev 2
> 4430: DSS rev 4.0, DISPC rev 4.0, DSI rev 3.0, RFBI rev 3.5
> 
> I haven't tried on OMAP2 yet..
> 
> Don't you think these revision numbers are enough to differentiate the
> features of each IP block?

Perhaps. The problem is, I don't know what the version numbers mean, ie.
when are they changed, what are the changes. I would hope you that you
could find some internal info inside TI that would explain the
differences =).

We can of course reverse engineer the version numbers, and hope that we
decipher them correctly. For OMAP3430/3630/4430 the differences look
clear.

But how about OMAP rev changes? For example, at some 3430 revision the
bitfield lengths of video timing registers were changed. Does it show on
DSS/DISPC version numbers? I don't think I have boards with those revs,
so I can't check.

 Tomi

RE: DSS2 patch series

[Taneja, Archit]

Hi,

> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com] 
> Sent: Thursday, August 05, 2010 1:40 PM
> To: Taneja, Archit
> Cc: Semwal, Sumit; linux-omap@vger.kernel.org
> Subject: RE: DSS2 patch series
> 
> On Thu, 2010-08-05 at 09:06 +0200, ext Taneja, Archit wrote:
> > Hi,
> > 
> > > 
> > > Also, we should think how to reduce if (cpu_is_omap44xx()) lines. 
> > > There should be some kind of DSS capability list somewhere, which 
> > > would tell the features available. I haven't thought this 
> more, but 
> > > it'd be very nice if we could use the DSS HW version number to 
> > > decide what features there are.
> > > 
> > > However, TI answered that information about DSS HW 
> version numbers 
> > > is not available, and thus cannot be used =(. Perhaps you 
> could try 
> > > to dig out some information from inside TI?
> > > 
> > 
> > I read the DSS_REVISON, DISPC_REVISION etc registers on 
> 3430, 3630, 4430:
> > 
> > 3430: DSS rev 2.0, DISPC rev 3.0, RFBI rev 1.0, DSI rev 
> 1.0, VENC rev 
> > 2
> > 3630: DSS rev 2.0, DISPC rev 3.0, RFBI rev 1.0, DSI rev 
> 1.0, VENC rev 
> > 2
> > 4430: DSS rev 4.0, DISPC rev 4.0, DSI rev 3.0, RFBI rev 3.5
> > 
> > I haven't tried on OMAP2 yet..
> > 
> > Don't you think these revision numbers are enough to 
> differentiate the 
> > features of each IP block?
> 
> Perhaps. The problem is, I don't know what the version 
> numbers mean, ie.
> when are they changed, what are the changes. I would hope you 
> that you could find some internal info inside TI that would 
> explain the differences =).
[Archit] I have collected some information about what these revision
numbers mean from the TI folks. The following is what I have gathered:

-For each broad version of OMAP, like OMAP3430, OMAP3630, OMAP4430 and so on,
there is an independent revision list. These are changed/incremented when
the corresponding IP blocks are modified. The numbers which we see are probably
the ones which were chosen to put into the silicon.

So, it is possible that the revision numbers of ES_1 of OMAP3430 is exactly the
same as the ES_1 of OMAP3630 even if the IP blocks have changed. This is what is
seen in the prints of the revision of 3430 and 3630 I sent in the previous mail.

These revision numbers are hence useful only within the revisions of a particular
OMAP. It looks like that there is no single revision chain since OMAP2.

-After discussions with more TI DSS folks, it seems that some changes that we may
need to make in DSS software may not be dependent on the DSS hardware at all. For example,
the patch "OMAP3630:DSS2: Updating MAX divider value" was introduced because of a change
in PRCM.

So it seems that we will need to have omap2, omap3 and omap4 checks , best we can
do is prevent them from scattering around, i.e have them at a single place during
initialization.

How do you think we can clean things up?

Archit

> 
> We can of course reverse engineer the version numbers, and 
> hope that we decipher them correctly. For OMAP3430/3630/4430 
> the differences look clear.
> 
> But how about OMAP rev changes? For example, at some 3430 
> revision the bitfield lengths of video timing registers were 
> changed. Does it show on DSS/DISPC version numbers? I don't 
> think I have boards with those revs, so I can't check.
> 
>  Tomi
> 
> 
> 

RE: DSS2 patch series

[Tomi Valkeinen]

On Tue, 2010-08-10 at 11:33 +0200, ext Taneja, Archit wrote:
> [Archit] I have collected some information about what these revision
> numbers mean from the TI folks. The following is what I have gathered:
> 
> -For each broad version of OMAP, like OMAP3430, OMAP3630, OMAP4430 and so on,
> there is an independent revision list. These are changed/incremented when
> the corresponding IP blocks are modified. The numbers which we see are probably
> the ones which were chosen to put into the silicon.
> 
> So, it is possible that the revision numbers of ES_1 of OMAP3430 is exactly the
> same as the ES_1 of OMAP3630 even if the IP blocks have changed. This is what is
> seen in the prints of the revision of 3430 and 3630 I sent in the previous mail.
> 
> These revision numbers are hence useful only within the revisions of a particular
> OMAP. It looks like that there is no single revision chain since OMAP2.
> 
> -After discussions with more TI DSS folks, it seems that some changes that we may
> need to make in DSS software may not be dependent on the DSS hardware at all. For example,
> the patch "OMAP3630:DSS2: Updating MAX divider value" was introduced because of a change
> in PRCM.
> 
> So it seems that we will need to have omap2, omap3 and omap4 checks , best we can
> do is prevent them from scattering around, i.e have them at a single place during
> initialization.

Ok. Well, good that it's clear now =).

> How do you think we can clean things up?

If I remember right, there's some kind of feature framework being worked
on (or ready?), but I haven't looked at that at all. That may or may not
suit our needs.

But perhaps we could just have a separate dss_features.c file, which
would contain a bunch of functions that can be used to ask whether a
certain feature is supported, and also to ask certain values (max
dividers or similar).

 Tomi

RE: DSS2 patch series

[Taneja, Archit]

Hi,

> Ok. Well, good that it's clear now =).
> 
> > How do you think we can clean things up?
> 
> If I remember right, there's some kind of feature framework 
> being worked on (or ready?), but I haven't looked at that at 
> all. That may or may not suit our needs.
> 
> But perhaps we could just have a separate dss_features.c 
> file, which would contain a bunch of functions that can be 
> used to ask whether a certain feature is supported, and also 
> to ask certain values (max dividers or similar).

I talked to some more folks about this. To summarize:

- The revision registers aren't reliable enough, it's better to
use the combination of cpu_is_xxxx and and omap_rev macros. These
should be enough for making an DSS specific feature list.

-The feature framework(HWMOD) can help out with the following things
	- The internal IP blocks within DSS.
	- The PRCM clocks and IRQs coming to DSS, and PM stuff which I don't
	know much about.
	- Effectively, the information on how the outside world communicates with DSS.

-DSS features like number of vid pipelines, supported color modes,internal clocks
and PLL info, bit fields needs to be managed by us.

One good input was that we can manage internal DSS clocks using the exisiting
clock framework and custom clock modes. I don't know much about it. Others
in the list can probably help out with this :)

The present way of handling DSS2 clocks is good, but we need to see if it can be
scalable when more OMAPs come in.

The dss_features.c idea sounds good, we will still have these new bunch of functions
scattered around in the code. But it will be much than an if else chain of omap checks.

So should we stick with this idea?

Thanks,

Archit

Re: DSS2 patch series

[Cousson, Benoit]

Hi Archit,

On 8/17/2010 1:16 PM, Taneja, Archit wrote:
> Hi,
>
>> Ok. Well, good that it's clear now =).
>>
>>> How do you think we can clean things up?
>>
>> If I remember right, there's some kind of feature framework
>> being worked on (or ready?), but I haven't looked at that at
>> all. That may or may not suit our needs.
>>
>> But perhaps we could just have a separate dss_features.c
>> file, which would contain a bunch of functions that can be
>> used to ask whether a certain feature is supported, and also
>> to ask certain values (max dividers or similar).
>
> I talked to some more folks about this. To summarize:
>
> - The revision registers aren't reliable enough, it's better to
> use the combination of cpu_is_xxxx and and omap_rev macros. These
> should be enough for making an DSS specific feature list.
>
> -The feature framework(HWMOD) can help out with the following things
> 	- The internal IP blocks within DSS.
> 	- The PRCM clocks and IRQs coming to DSS, and PM stuff which I don't
> 	know much about.
> 	- Effectively, the information on how the outside world communicates with DSS.
>
> -DSS features like number of vid pipelines, supported color modes,internal clocks
> and PLL info, bit fields needs to be managed by us.

You can use hwmod to store that as well. Other IPs might require 
features management, so instead of doing a DSS dedicated solution, you 
can directly leverage the existing fmwk and extend it to manage your 
features.

> One good input was that we can manage internal DSS clocks using the exisiting
> clock framework and custom clock modes. I don't know much about it. Others
> in the list can probably help out with this :)
>
> The present way of handling DSS2 clocks is good, but we need to see if it can be
> scalable when more OMAPs come in.

Good? It works, but this is still redoing what the clocks framework can 
already do. The good approach will be to encode your clocks nodes using 
the clock framework, as you've just said.

> The dss_features.c idea sounds good, we will still have these new bunch of functions
> scattered around in the code. But it will be much than an if else chain of omap checks.
>
> So should we stick with this idea?

Using directly the hwmod struct sound a much better idea for my point of 
view.

Regards,
Benoit

RE: DSS2 patch series

[Tomi Valkeinen]

On Tue, 2010-08-17 at 13:16 +0200, ext Taneja, Archit wrote:
> Hi,
> 
> > Ok. Well, good that it's clear now =).
> > 
> > > How do you think we can clean things up?
> > 
> > If I remember right, there's some kind of feature framework 
> > being worked on (or ready?), but I haven't looked at that at 
> > all. That may or may not suit our needs.
> > 
> > But perhaps we could just have a separate dss_features.c 
> > file, which would contain a bunch of functions that can be 
> > used to ask whether a certain feature is supported, and also 
> > to ask certain values (max dividers or similar).
> 
> I talked to some more folks about this. To summarize:
> 
> - The revision registers aren't reliable enough, it's better to
> use the combination of cpu_is_xxxx and and omap_rev macros. These
> should be enough for making an DSS specific feature list.
> 
> -The feature framework(HWMOD) can help out with the following things
> 	- The internal IP blocks within DSS.
> 	- The PRCM clocks and IRQs coming to DSS, and PM stuff which I don't
> 	know much about.
> 	- Effectively, the information on how the outside world communicates with DSS.
> 
> -DSS features like number of vid pipelines, supported color modes,internal clocks
> and PLL info, bit fields needs to be managed by us.
> 
> One good input was that we can manage internal DSS clocks using the exisiting
> clock framework and custom clock modes. I don't know much about it. Others
> in the list can probably help out with this :)
> 
> The present way of handling DSS2 clocks is good, but we need to see if it can be
> scalable when more OMAPs come in.
> 
> The dss_features.c idea sounds good, we will still have these new bunch of functions
> scattered around in the code. But it will be much than an if else chain of omap checks.
> 
> So should we stick with this idea?

Yes, and even if the dss_features.c isn't what is needed in the end,
it'll still be easier to convert to whatever way we want in the future.

But this is also not a very high priority thing. So I don't see a need
to start converting everything to use dss_features.c right away. We can
start by converting the places where OMAP4 changes require feature
checks.

 Tomi

RE: DSS2 patch series

[Taneja, Archit]

Hi Benoit,

Thanks for the comments, ill study how we can leverage the hwmod fw for DSS features.

Archit

> -----Original Message-----
> From: Cousson, Benoit 
> Sent: Tuesday, August 17, 2010 5:03 PM
> To: Taneja, Archit
> Cc: Tomi Valkeinen; Semwal, Sumit; 
> linux-omap@vger.kernel.org; Paul Walmsley; Kevin Hilman
> Subject: Re: DSS2 patch series
> 
> Hi Archit,
> 
> On 8/17/2010 1:16 PM, Taneja, Archit wrote:
> > Hi,
> >
> >> Ok. Well, good that it's clear now =).
> >>
> >>> How do you think we can clean things up?
> >>
> >> If I remember right, there's some kind of feature framework being 
> >> worked on (or ready?), but I haven't looked at that at 
> all. That may 
> >> or may not suit our needs.
> >>
> >> But perhaps we could just have a separate dss_features.c 
> file, which 
> >> would contain a bunch of functions that can be used to ask 
> whether a 
> >> certain feature is supported, and also to ask certain values (max 
> >> dividers or similar).
> >
> > I talked to some more folks about this. To summarize:
> >
> > - The revision registers aren't reliable enough, it's better to use 
> > the combination of cpu_is_xxxx and and omap_rev macros. 
> These should 
> > be enough for making an DSS specific feature list.
> >
> > -The feature framework(HWMOD) can help out with the following things
> > 	- The internal IP blocks within DSS.
> > 	- The PRCM clocks and IRQs coming to DSS, and PM stuff 
> which I don't
> > 	know much about.
> > 	- Effectively, the information on how the outside world 
> communicates with DSS.
> >
> > -DSS features like number of vid pipelines, supported color 
> > modes,internal clocks and PLL info, bit fields needs to be 
> managed by us.
> 
> You can use hwmod to store that as well. Other IPs might 
> require features management, so instead of doing a DSS 
> dedicated solution, you can directly leverage the existing 
> fmwk and extend it to manage your features.
> 
> > One good input was that we can manage internal DSS clocks using the 
> > exisiting clock framework and custom clock modes. I don't know much 
> > about it. Others in the list can probably help out with this :)
> >
> > The present way of handling DSS2 clocks is good, but we 
> need to see if 
> > it can be scalable when more OMAPs come in.
> 
> Good? It works, but this is still redoing what the clocks 
> framework can already do. The good approach will be to encode 
> your clocks nodes using the clock framework, as you've just said.
> 
> > The dss_features.c idea sounds good, we will still have these new 
> > bunch of functions scattered around in the code. But it 
> will be much than an if else chain of omap checks.
> >
> > So should we stick with this idea?
> 
> Using directly the hwmod struct sound a much better idea for 
> my point of view.
> 
> Regards,
> Benoit
> 

RE: DSS2 patch series

[Taneja, Archit]

Hi, 

> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com] 
> Sent: Tuesday, August 17, 2010 5:10 PM
> To: Taneja, Archit
> Cc: Semwal, Sumit; linux-omap@vger.kernel.org
> Subject: RE: DSS2 patch series
> 
> On Tue, 2010-08-17 at 13:16 +0200, ext Taneja, Archit wrote:
> > Hi,
> > 
> > > Ok. Well, good that it's clear now =).
> > > 
> > > > How do you think we can clean things up?
> > > 
> > > If I remember right, there's some kind of feature framework being 
> > > worked on (or ready?), but I haven't looked at that at 
> all. That may 
> > > or may not suit our needs.
> > > 
> > > But perhaps we could just have a separate dss_features.c 
> file, which 
> > > would contain a bunch of functions that can be used to 
> ask whether a 
> > > certain feature is supported, and also to ask certain values (max 
> > > dividers or similar).
> > 
> > I talked to some more folks about this. To summarize:
> > 
> > - The revision registers aren't reliable enough, it's better to use 
> > the combination of cpu_is_xxxx and and omap_rev macros. 
> These should 
> > be enough for making an DSS specific feature list.
> > 
> > -The feature framework(HWMOD) can help out with the following things
> > 	- The internal IP blocks within DSS.
> > 	- The PRCM clocks and IRQs coming to DSS, and PM stuff 
> which I don't
> > 	know much about.
> > 	- Effectively, the information on how the outside world 
> communicates with DSS.
> > 
> > -DSS features like number of vid pipelines, supported color 
> > modes,internal clocks and PLL info, bit fields needs to be 
> managed by us.
> > 
> > One good input was that we can manage internal DSS clocks using the 
> > exisiting clock framework and custom clock modes. I don't know much 
> > about it. Others in the list can probably help out with this :)
> > 
> > The present way of handling DSS2 clocks is good, but we 
> need to see if 
> > it can be scalable when more OMAPs come in.
> > 
> > The dss_features.c idea sounds good, we will still have these new 
> > bunch of functions scattered around in the code. But it 
> will be much than an if else chain of omap checks.
> > 
> > So should we stick with this idea?
> 
> Yes, and even if the dss_features.c isn't what is needed in 
> the end, it'll still be easier to convert to whatever way we 
> want in the future.
> 
> But this is also not a very high priority thing. So I don't 
> see a need to start converting everything to use 
> dss_features.c right away. We can start by converting the 
> places where OMAP4 changes require feature checks.
> 
>  Tomi
> 

Okay, I'll come up with a way to implement the dss_features.c idea,
I'll also try to learn how we can use hwmod for dss specific features.

Archit