On Fri, Jan 25, 2013 at 7:19 AM, Mohammed, Afzal <afzal@xxxxxx> wrote: > Hi Rob, > > On Wed, Jan 23, 2013 at 04:06:22, Rob Clark wrote: > >> A simple DRM/KMS driver for the TI LCD Controller found in various >> smaller TI parts (AM33xx, OMAPL138, etc). This driver uses the > >> +void tilcdc_crtc_update_clk(struct drm_crtc *crtc) > >> + /* in raster mode, minimum divisor is 2: */ >> + ret = clk_set_rate(priv->disp_clk, crtc->mode.clock * 1000 * 2); > > These things can better be handled with divider clock having a > minimum value (being discussed with Mike on how exactly it should > be) and instead of setting rate over a platform specific clock, > you can set rate over lcd clock using SET_RATE_PARENT at platform > level, more below, I looked at that patch you were proposing for da8xx-fb.. to be honest, it did not seem simpler to me, so I was sort of failing to see the benefit.. >> + /* Configure the LCD clock divisor. */ >> + tilcdc_write(dev, LCDC_CTRL_REG, LCDC_CLK_DIVISOR(div) | >> + LCDC_RASTER_MODE); >> + >> + if (priv->rev == 2) >> + tilcdc_set(dev, LCDC_CLK_ENABLE_REG, >> + LCDC_V2_DMA_CLK_EN | LCDC_V2_LIDD_CLK_EN | >> + LCDC_V2_CORE_CLK_EN); > > Mike was suggesting to model the above using gate/divider/composite > clocks of CCF in the FB driver. > >> + priv->clk = clk_get(dev->dev, "fck"); >> + if (IS_ERR(priv->clk)) { >> + dev_err(dev->dev, "failed to get functional clock\n"); >> + ret = -ENODEV; >> + goto fail; >> + } >> + >> + priv->disp_clk = clk_get(dev->dev, "dpll_disp_ck"); >> + if (IS_ERR(priv->clk)) { >> + dev_err(dev->dev, "failed to get display clock\n"); >> + ret = -ENODEV; >> + goto fail; >> + } > > "dpll_disp_ck" is a platform specific matter, driver should not > be handling platform specifics. And this won't work on DaVinci, > you can probably make use of meaning the clock has a different name on davinci, or? Presumably there must be some clock generating the pixel clock for the display, but I confess to not being too familiar with the details on davinci.. BR, -R > my series "[PATCH v2 0/4] ARM: AM335x: LCDC platform support" > > or something similar to overcome this. > > Regards > Afzal > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel