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

[Archit Taneja <archit@ti.com>]

On Friday 01 April 2011 02:09 PM, Valkeinen, Tomi wrote:
> 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.

Yeah it is, sure.

Archit