On Wed, Jan 24, 2018 at 02:07:30AM +0200, Laurent Pinchart wrote:
> Hi Ville,
>
> Thank you for the patch.
>
> On Tuesday, 23 January 2018 19:08:53 EET Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Use drm_mode_get_hv_timing() to fill out the plane clip rectangle.
> >
> > No functional changes as the code already uses crtc_state->mode
> > to populate the clip, which is also what drm_mode_get_hv_timing()
> > uses.
> >
> > Once everyone agrees on this we can move the clip handling into
> > drm_atomic_helper_check_plane_state().
> >
> > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 4a3d16cf3ed6..5687a94d4cb1
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > @@ -572,7 +572,7 @@ int __rcar_du_plane_atomic_check(struct drm_plane
> > *plane, {
> > struct drm_device *dev = plane->dev;
> > struct drm_crtc_state *crtc_state;
> > - struct drm_rect clip;
> > + struct drm_rect clip = {};
>
> Nitpicking, isn't the correct C99 zero initializer { 0 } ? I doesn't matter
> too much as the variable is removed in patch 5/5 but we could as well fix it
> if needed.
{ 0 } is annying because it can cause warnings if the first member is
not an integer or whatever. Also it could be confused with someone
actually wanting to zero initialize the first member explicitly (and
not knowing about named initializers). So I think {} conveys the
meaning much more clearly as well.
>
> Apart from that,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> > int ret;
> >
> > if (!state->crtc) {
> > @@ -589,10 +589,9 @@ int __rcar_du_plane_atomic_check(struct drm_plane
> > *plane, if (IS_ERR(crtc_state))
> > return PTR_ERR(crtc_state);
> >
> > - clip.x1 = 0;
> > - clip.y1 = 0;
> > - clip.x2 = crtc_state->mode.hdisplay;
> > - clip.y2 = crtc_state->mode.vdisplay;
> > + if (crtc_state->enable)
> > + drm_mode_get_hv_timing(&crtc_state->mode,
> > + &clip.x2, &clip.y2);
> >
> > ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
> > DRM_PLANE_HELPER_NO_SCALING,
>
> --
> Regards,
>
> Laurent Pinchart
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
