Re: [PATCH 1/9] OMAP: DSS2: move dss device clock configuration

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

On Fri, 2011-04-01 at 13:49 +0530, Archit Taneja wrote:

> But there are some parameters which might get common across dss devices.
> Things like dispc clock source, dispc core clock divider will be shared 
> across the panels. We had discussed the possibility of declaring this 
> common info in omap_dss_board_info or as a separate common_clocks 
> structure. Each device could pick this filled up common_clocks struct, 
> or fill it up its own way (for the use cases which has only one panel on 
> at a time). I was wondering if it would be easy to move to this approach 
> with your patch. dispc itself would now have some common clock stuff and 
> per panel clock stuff. Is that a very clean approach?

I don't know =). I think the simplest solution is to have full divisor
info for each dss_device. And it's up to the board file writer to make
sure the divisors match for all the displays that can be enabled at the
same time.

I don't think that is perfect, but trying to share the data sounds a bit
confusing. Especially as there are just a divisor and a clock source
that are shared. If we have a lot of common data, then a shared struct
would of course be better.

> Anyway, I think it would be good to have a channel struct, as there are 
> more things to put in dispc clocks, it should look something like:
> 
> struct clocks {
> 		struct {
> 			struct {
> 				u16 lck_div;
> 				u16 pck_div;
> 				enum clock_source lcd_clk_src;
> 			} channel;
> 			...
> 			...
> 			u16 core_clk_div;
> 		} dispc;
> 		...
> 		...
> };

In my original patch I had:

        struct {
                struct {
                        u16 fck_div;
                } dss;

                struct {
                        u16 lck_div;
                        u16 pck_div;

                        bool fclk_from_dsi_pll;
                } dispc;

		...
	};

Which I removed due to comments and slight confusion how to handle the
DSS_FCLK divisor and the clock source.

Adding the clock source there needs some more work, moving the enum to
public include file, and implementing the support. If you agree that
this patch in its current form is an improvement, I'd like to go forward
with this and work on the clock source later.

 Tomi