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@linux.intel.com
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@ideasonboard.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
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@ideasonboard.com
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