[PATCH v13 04/14] drm/mediatek: Add DPI sub driver

[Philipp Zabel]

Hi Daniel,

Am Mittwoch, den 09.03.2016, 22:23 +0800 schrieb Daniel Kurtz:
> Hi Philipp, Jie,
> 
> Some small comments.
> Nothing that can't be fixed after merging if you prefer.
>
> On Tue, Mar 8, 2016 at 9:27 PM, Philipp Zabel <p.zabel at pengutronix.de> wrote:
[...]
> > +static int mtk_dpi_power_on(struct mtk_dpi *dpi, enum mtk_dpi_power_ctl pctl)
> > +{
> > +       int ret;
> > +
> > +       dpi->power_ctl |= pctl;
> > +
> > +       if (!(dpi->power_ctl & DPI_POWER_START) &&
> > +           !((dpi->power_ctl & DPI_POWER_ENABLE) &&
> > +             ((dpi->power_ctl & DPI_POWER_RESUME))))
> 
> There one too many () on the POWER_RESUME clause.

I'll try to remove the suspend ops and this with them.

[...]
> > +static SIMPLE_DEV_PM_OPS(mtk_dpi_pm_ops, mtk_dpi_suspend, mtk_dpi_resume);
> 
> Like hdmi & dsi, I think we can remove these PM routines.
> And maybe then we can remove the "DPI_POWER_RESUME" logic (which I
> don't grok yet)?

Right, also we probably should use drm_atomic_helper_suspend/resume in
mtk_drm_sys_suspend/resume.

[...]
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.h b/drivers/gpu/drm/mediatek/mtk_dpi.h
> > new file mode 100644
> > index 0000000..4fa4114
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.h
> 
> All of this can be moved to mtk_dpi.c.

Ok, I'll merge this into mtk_dpi.c.

regards
Philipp