On Mon, 5 Jul 2010, Magnus Damm wrote:
> On Wed, Jun 30, 2010 at 6:55 PM, Guennadi Liakhovetski
> <g.liakhovetski@xxxxxx> wrote:
> > Support HDMI in 720p mode.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> > ---
> > ═arch/arm/mach-shmobile/board-ap4evb.c | ═128 ++++++++++++++++++++++++++++++++-
> > ═1 files changed, 127 insertions(+), 1 deletions(-)
>
> Hi Guennadi,
>
> Thanks for your work on this!
>
> > --- a/arch/arm/mach-shmobile/board-ap4evb.c
> > +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> > @@ -645,8 +732,36 @@ static void __init gpio_no_direction(u32 addr)
> > ═#define GPIO_PORT9CR ═ 0xE6051009
> > ═#define GPIO_PORT10CR ═0xE605100A
> >
> > +static int __init hdmi_init_pm_clock(void)
> > +{
> > + ═ ═ ═ struct clk *hdmi_pm = clk_get(NULL, "sh-hdmi.0"),
> > + ═ ═ ═ ═ ═ ═ ═ *lcdc1_pm = clk_get(&lcdc1_device.dev, "sh_mobile_lcdc_fb.1");
> > + ═ ═ ═ int ret;
> > +
> > + ═ ═ ═ if (!IS_ERR(hdmi_pm)) {
> > + ═ ═ ═ ═ ═ ═ ═ ret = clk_enable(hdmi_pm);
> > + ═ ═ ═ ═ ═ ═ ═ if (ret < 0)
> > + ═ ═ ═ ═ ═ ═ ═ ═ ═ ═ ═ pr_err("Cannot enable clock: %d\n", ret);
> > + ═ ═ ═ } else {
> > + ═ ═ ═ ═ ═ ═ ═ pr_err("Cannot get HDMI PM: %ld\n", PTR_ERR(hdmi_pm));
> > + ═ ═ ═ }
> > +
> > + ═ ═ ═ if (!IS_ERR(lcdc1_pm)) {
> > + ═ ═ ═ ═ ═ ═ ═ ret = clk_enable(lcdc1_pm);
> > + ═ ═ ═ ═ ═ ═ ═ if (ret < 0)
> > + ═ ═ ═ ═ ═ ═ ═ ═ ═ ═ ═ pr_err("Cannot enable clock: %d\n", ret);
> > + ═ ═ ═ } else {
> > + ═ ═ ═ ═ ═ ═ ═ pr_err("Cannot get LCDC1 PM: %ld\n", PTR_ERR(lcdc1_pm));
> > + ═ ═ ═ }
> > +
> > + ═ ═ ═ return 0;
> > +}
>
> This part I'm not so happy with.
>
> Question 1:
>
> Why do you need to statically enable the LCDC clock here? I guess it
> works around the fact that Runtime PM is missing at this point? The
> LCDC driver should manage its own clocks. With this in place the
> clocks will never be disabled and your Runtime PM changes to the LCDC
> driver will never actually stop the clocks.
Not yet on ARM, no. The change to the lcdcfb runtime-pm, that actually
does anything at all with HDMI on ap4evb is the addition of ->display_on
and ->display_off calls.
> Question 2:
>
> And why do we need to enable the HDMI clock here? Same reason as
> above? I imagine that the HDMI driver already does Runtime PM?
Exactly, just like with MIPI (see ap4evb_init_display_clk()).
> Also, you should not need to pass any strings to clk_get(). clkdev
> should be able to figure out the binding from the device name.
Ok
Thanks
Guennadi
---
Guennadi Liakhovetski
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
