Hi Martin,
Am Mittwoch, den 27.03.2013, 19:40 +0100 schrieb Martin Fuzzey:
> Hi Philipp,
>
> On 27/03/13 18:30, Philipp Zabel wrote:
> > +static bool imx_ldb_encoder_mode_fixup(struct drm_encoder *encoder,
> > + const struct drm_display_mode *mode,
> > + struct drm_display_mode *adjusted_mode)
> > +{
> > +/*
> > + struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
> > +
> > + adjusted_mode->clock = clk_round_rate(imx_ldb_ch->ldb->clk_pll[imx_ldb_ch->chno],
> > + adjusted_mode->clock * 1000) / 1000;
> > +*/
> This should probably be removed :)
Right.
> > +static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno,
> > + unsigned long serial_clk, unsigned long di_clk)
> > +{
> > + int ret;
> > +
> > + dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
> > + clk_get_rate(ldb->clk_pll[chno]), serial_clk);
> > + clk_set_rate(ldb->clk_pll[chno], serial_clk);
> > +
> > + dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
> > + clk_get_rate(ldb->clk_pll[chno]));
> > +
> > + dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
> > + clk_get_rate(ldb->clk[chno]),
> > + (long int)di_clk);
> > + clk_set_rate(ldb->clk[chno], di_clk);
> > +
> > + dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
> > + clk_get_rate(ldb->clk[chno]));
> > +
> Are all these debug statements still useful?
The clocking is a hairy, especially if the board code decided to use an
unexpected PLL as source for the LVDS serial clock. But I agree this
could be condensed a bit.
> > +static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
> > +{
> > + if (imx_ldb_ch == &ldb->channel[0] || dual) {
> > + ldb->ldb_ctrl &= ~0x3;
> > + if (mux == 0 || ldb->lvds_mux)
> > + ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI0;
> > + else if (mux == 1)
> > + ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI1;
> > + }
> > + if (imx_ldb_ch == &ldb->channel[1] || dual) {
> > + ldb->ldb_ctrl &= ~0xc;
> > + if (mux == 1 || ldb->lvds_mux)
> > + ldb->ldb_ctrl |= LDB_CH1_MODE_EN_TO_DI1;
> > + else if (mux == 0)
> > + ldb->ldb_ctrl |= LDB_CH1_MODE_EN_TO_DI0;
> > + }
> Maybe avoid the magic 0x3 and 0x0c by building from LDB_CHx_MODE_EN_TO_DIx?
Ok, I'll prune the magic constants.
> > +
> > +static void imx_ldb_encoder_disable(struct drm_encoder *encoder)
> > +{
> > + struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
> > + struct imx_ldb *ldb = imx_ldb_ch->ldb;
> > +
> > + /*
> > + * imx_ldb_encoder_disable is called by
> > + * drm_helper_disable_unused_functions without
> > + * the encoder being enabled before.
> > + */
> > + if (imx_ldb_ch == &ldb->channel[0] && (ldb->ldb_ctrl & 0x3) == 0)
> > + return;
> > + else if (imx_ldb_ch == &ldb->channel[1] && (ldb->ldb_ctrl & 0xc) == 0)
> > + return;
> > +
> > + if (imx_ldb_ch == &ldb->channel[0])
> > + ldb->ldb_ctrl &= ~0x3;
> > + else if (imx_ldb_ch == &ldb->channel[1])
> > + ldb->ldb_ctrl &= ~0xc;
> > +
> Idem magic numbers
thanks
Philipp
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
