Re: kernel panic with latest DSS

From: Steve Sakoman <sakoman@gmail.com>
To: Tomi.Valkeinen@nokia.com, linux-omap@vger.kernel.org
Subject: Re: kernel panic with latest DSS
Date: Wed, 10 Mar 2010 11:48:37 -0800	[thread overview]
Message-ID: <5e088bd91003101148o82a07e2j4df803d23ee78d83@mail.gmail.com> (raw)
In-Reply-To: <3647DBDA80551C4696E9E4506F0BE08E255DC2F2A7@NOK-EUMSG-02.mgdnok.nokia.com>

On Wed, Mar 10, 2010 at 10:58 AM,  <Tomi.Valkeinen@nokia.com> wrote:

> I haven't had time to look at this, but I don't think your analysis is correct.

You are correct of course.  My sleepy eyes neglected to notice that
double = and that makes a big difference :-)

> What first comes to my mind is that dss.dispc_clk_source is, for some reason, uninitialized. Could you put prints to dss_select_dispc_clk_source to see if it's called, and if so, what is assigned to dss.dispc_clk_source?

I did a build with that change and it does seem that
dss_select_dispc_clk_source is never called.

Steve

>  Tomi
>
> ________________________________________
> From: ext Steve Sakoman [sakoman@gmail.com]
> Sent: Wednesday, March 10, 2010 20:42
> To: linux-omap@vger.kernel.org; Valkeinen Tomi (Nokia-D/Helsinki)
> Subject: Re: kernel panic with latest DSS
>
> On Wed, Mar 10, 2010 at 10:16 AM, Steve Sakoman <sakoman@gmail.com> wrote:
>
>> The issue occurs in the following section of code in
>> drivers/video/omap2/dss/dispc.c:
>>
>> unsigned long dispc_fclk_rate(void)
>> {
>>        unsigned long r = 0;
>>
>>        if (dss_get_dispc_clk_source() == DSS_SRC_DSS1_ALWON_FCLK)
>>                r = dss_clk_get_rate(DSS_CLK_FCK1);
>>        else
>> #ifdef CONFIG_OMAP2_DSS_DSI
>>                r = dsi_get_dsi1_pll_rate();
>> #else
>>        BUG();
>> #endif
>>        return r;
>> }
>>
>> Since my machines don't have DSI displays CONFIG_OMAP2_DSS_DSI is not
>> defined and the BUG() call is triggered.
>
> The issue seems to be the test for DSS_SRC_DSS1_ALWON_FCLK.
>
> Looking at  drivers/video/omap2/dss/dispc.c I see that
> DSS_SRC_DSS1_ALWON_FCLK will never be returned since clk_src is always
> set to 0 or 1 based on a test of DSS_SRC_DSS1_ALWON_FCLK :
>
> void dss_select_dispc_clk_source(enum dss_clk_source clk_src)
> {
>        int b;
>
>        BUG_ON(clk_src != DSS_SRC_DSI1_PLL_FCLK &&
>                        clk_src != DSS_SRC_DSS1_ALWON_FCLK);
>
>        b = clk_src == DSS_SRC_DSS1_ALWON_FCLK ? 0 : 1;
>
>        REG_FLD_MOD(DSS_CONTROL, b, 0, 0);      /* DISPC_CLK_SWITCH */
>
>        dss.dispc_clk_source = clk_src;
> }
>
> It seems to me that the test should return to the way it was in the
> previous DSS version, i.e. a test for 0:
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index e777e35..be9b3de 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -2218,7 +2218,7 @@ unsigned long dispc_fclk_rate(void)
>  {
>        unsigned long r = 0;
>
> -       if (dss_get_dispc_clk_source() == DSS_SRC_DSS1_ALWON_FCLK)
> +       if (dss_get_dispc_clk_source() == 0)
>                r = dss_clk_get_rate(DSS_CLK_FCK1);
>        else
>  #ifdef CONFIG_OMAP2_DSS_DSI
>
> A test build confirms that this change restores my ability to play
> DVD's and display webcam streams.  My understanding of this code is
> superficial, so hopefully Tomi can comment on whether this is a good
> fix or not.
>
> Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html