On Thu, Apr 19, 2012 at 6:41 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> On Mon, 2012-04-02 at 20:43 +0530, Chandrabhanu Mahapatra wrote:
>> DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
>> calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
>> DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
>> divisor LCD.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@xxxxxx>
>> ---
>> drivers/video/omap2/dss/dispc.c | 28 ++++++++++++++++++++++------
>> drivers/video/omap2/dss/dss.h | 1 +
>> 2 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index 17ffa71..cfde674 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -1813,6 +1813,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>> dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
>> const int max_decim_limit = 16;
>> unsigned long fclk = 0;
>> + unsigned long dispc_core_clk = dispc_core_clk_rate(channel);
>> int decim_x, decim_y, error, min_factor;
>> u16 in_width, in_height, in_width_max = 0;
>>
>> @@ -1855,7 +1856,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>> fclk = calc_fclk(channel, in_width, in_height,
>> out_width, out_height);
>> error = (in_width > maxsinglelinewidth || !fclk ||
>> - fclk > dispc_fclk_rate());
>> + fclk > dispc_core_clk);
>> if (error) {
>> if (decim_x == decim_y) {
>> decim_x = min_factor;
>> @@ -1893,7 +1894,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>> out_width, out_height);
>> error = (error || in_width > maxsinglelinewidth * 2 ||
>> (in_width > maxsinglelinewidth && *five_taps) ||
>> - !fclk || fclk > dispc_fclk_rate());
>> + !fclk || fclk > dispc_core_clk);
>> if (error) {
>> if (decim_x == decim_y) {
>> decim_x = min_factor;
>> @@ -1926,7 +1927,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>> } else {
>> int decim_x_min = decim_x;
>> in_height = DIV_ROUND_UP(height, decim_y);
>> - in_width_max = dispc_fclk_rate() /
>> + in_width_max = dispc_core_clk /
>> DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
>> out_width);
>> decim_x = DIV_ROUND_UP(width, in_width_max);
>> @@ -1950,13 +1951,13 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>> }
>>
>> DSSDBG("required fclk rate = %lu Hz\n", fclk);
>> - DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
>> + DSSDBG("current fclk rate = %lu Hz\n", dispc_core_clk);
>>
>> - if (!fclk || fclk > dispc_fclk_rate()) {
>> + if (!fclk || fclk > dispc_core_clk) {
>> DSSERR("failed to set up scaling, "
>> "required fclk rate = %lu Hz, "
>> "current fclk rate = %lu Hz\n",
>> - fclk, dispc_fclk_rate());
>> + fclk, dispc_core_clk);
>> return -EINVAL;
>> }
>>
>> @@ -2646,6 +2647,21 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
>> }
>> }
>>
>> +unsigned long dispc_core_clk_rate(enum omap_channel channel)
>> +{
>> + int lcd = 1;
>> + unsigned long r = dispc_fclk_rate();
>> +
>> + if (dss_has_feature(FEAT_CORE_CLK_DIV)) {
>> + lcd = REG_GET(DISPC_DIVISOR, 23, 16);
>> + } else {
>> + if (dispc_mgr_is_lcd(channel))
>> + lcd = REG_GET(DISPC_DIVISORo(channel), 23, 16);
>> + }
>> +
>> + return r / lcd ;
>> +}
>> +
>
> I wonder if this is correct. "channel" for dispc core clock doesn't make
> sense, there's no channel related to that. At least on OMAP4.
>
> If I'm not mistaken, in omap2/3 case (i.e.
> dss_has_feature(FEAT_CORE_CLK_DIV) == false) we can just use channel 0
> to get the lcd divisor. Although that would mean that LCD output's
> divisor affects the tv-out's scaling calculations, which feels a bit
> strange...
>
Yes, you are right. I had failed to look at channel 0 or
OMAP_DSS_CHANNEL_LCD can simply be used.
> So... I don't know... Do we really have two different dispc core clocks
> int omap2/3 (for lcd output and for tv output), but only one in omap4?
> If so, then the above code is ok. Have you found explanations from TRM
> which say what clock is required and where?
>
Though the term Dispc Core Clock is never used in OMAP2/3 TRM, it is
actually the logic clock that drives the various logics of DSS
subsystem. So we can say that we have one dispc core clock for OMAP2/3
and that being the logic clock. I took two different logics as LCD
factor returned zero for TV. However, LCD factor of DISPC_DIVISOR of
channel 0 can be used always.
> In any case, please remove the initialization of lcd variable, and add:
>
> else
> lcd = 1;
>
> I think that's much clearer. And "r" variable is commonly used as a
> return value. I would rename the variable to something else, say, "fck".
>
> Tomi
>
Yes, sure it makes code more understandable and clean.
--
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
