[PATCH v4 3/3] drm: zte: add overlay plane support

[shawnguo@kernel.org (Shawn Guo)]

Hi Sean,

On Mon, Jan 09, 2017 at 11:27:44AM -0500, Sean Paul wrote:
> > +static inline void zx_osd_int_update(struct zx_crtc *zcrtc)
> > +{
> > +       struct zx_vou_hw *vou = zcrtc->vou;
> > +       int i;
> > +
> > +       vou_chn_set_update(zcrtc);
> > +       zx_plane_set_update(zcrtc->primary);
> > +
> > +       for (i = 0; i < VL_NUM; i++) {
> > +               struct drm_plane *overlay = vou->overlays[i];
> > +
> > +               if (overlay)
> > +                       zx_plane_set_update(overlay);
> > +       }
> 
> Hi Shawn,
> Thanks so much for revving this patch, it's looking really good. I
> just have one (1.5, really) suggestion.
> 
> I don't think we need to keep vou->overlays around. You should be able
> to loop through all the planes registered with drm core and use
> crtc->state->plane_mask to determine which are active for a given crtc
> (this would also encapsulate the zcrtc->primary update above).
> 
> I think you can also use if (plane->state->crtc) as your
> enable/disable check in zx_plane_set_update() and eliminate the new
> enabled flag.

Great.  Since I'm still quite new to DRM subsystem, such core
infrastructural usage guide are really helpful for me.  I tested the
changes as you suggested, and they worked just fine.  So happy to see my
code gets cleaned up further.

> I fully realize this was my suggestion, and I apologize
> for the churn. I'll try not to do reviews past midnight again :-)

I'm so grateful to you for helping review my code, especially in
midnight time :)

Shawn