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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
