Re: [RFC PATCH 1/3] drm: use flags instead of boolean in plane check

2020-09-16 Thread Daniel Vetter
On Wed, Sep 16, 2020 at 12:31:44PM +0200, Stefan Agner wrote:
> On 2020-09-11 10:50, Daniel Vetter wrote:
> > On Thu, Sep 10, 2020 at 11:24:23AM +0200, Stefan Agner wrote:
> >> To improve readability and make it easier to add further optional checks
> >> replace the boolean parameters with a single flag bitfield as parameter
> >> of drm_atomic_helper_check_plane_state.
> >>
> >> The regular call sites have been converted using a simple coccinelle
> >> patch.
> >>
> >> virtual patch
> > 
> > Looks reasonable, but needs kerneldoc. For that either do a list in the
> > kerneldoc for drm_atomic_helper_check_plane_state, or make the flags an
> > enum (still setting the value for each explicitly), and then kerneldoc
> > that with a link from the function to that.
> 
> The kerneldoc updates for drm_atomic_helper_check_plane_state() should
> cover it mostly. I did miss update it in patch 2, so will fix that.
> 
> I thought to keep it similar to drm_atomic_helper_commit_planes() and
> use #defines as well as describing the flags in the function kerneldoc.
> Should I make it a list instead?

A kerneldoc/rst list within the function should work well too and look
pretty. Whatever you prefer, just make sure the html output isn't broken
somehow.
-Daniel

> 
> --
> Stefan
> 
> > -Daniel
> > 
> >>
> >> @@
> >> expression e1, e2, e3, e4;
> >> symbol true, false;
> >> @@
> >>
> >> (
> >> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, true)
> >> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 
> >> DRM_PLANE_CAN_POSITION | DRM_PLANE_CAN_UPDATE_DISABLED)
> >> |
> >> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, false)
> >> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 
> >> DRM_PLANE_CAN_POSITION)
> >> |
> >> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, true)
> >> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 
> >> DRM_PLANE_CAN_UPDATE_DISABLED)
> >> |
> >> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, false)
> >> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 0)
> >> )
> >>
> >> Signed-off-by: Stefan Agner 
> >> ---
> >> This implements what has been discussed in the thread of the patch
> >> "drm: mxsfb: check framebuffer pitch":
> >> https://lkml.org/lkml/2020/9/8/1342
> >>
> >> Before sending it out to all maintainers I wanted to get conformation
> >> if the general approach is fine.
> >>
> >> I think in some places there should be another linebreak for the flags,
> >> but this is what coccinelle gives me. Not sure if I should manually fix
> >> those places...
> >>
> >> --
> >> Stefan
> >>
> >>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  5 ++--
> >>  drivers/gpu/drm/arm/hdlcd_crtc.c  |  6 ++---
> >>  drivers/gpu/drm/arm/malidp_planes.c   |  5 ++--
> >>  drivers/gpu/drm/armada/armada_plane.c |  3 ++-
> >>  drivers/gpu/drm/ast/ast_mode.c|  4 +--
> >>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   |  4 +--
> >>  drivers/gpu/drm/drm_atomic_helper.c   | 17 +++--
> >>  drivers/gpu/drm/drm_plane_helper.c|  9 +++
> >>  drivers/gpu/drm/drm_simple_kms_helper.c   |  2 +-
> >>  drivers/gpu/drm/i915/display/intel_display.c  |  9 ---
> >>  drivers/gpu/drm/i915/display/intel_sprite.c   |  6 ++---
> >>  drivers/gpu/drm/imx/ipuv3-plane.c |  7 --
> >>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c |  7 --
> >>  drivers/gpu/drm/mediatek/mtk_drm_plane.c  |  4 +--
> >>  drivers/gpu/drm/meson/meson_overlay.c |  2 +-
> >>  drivers/gpu/drm/meson/meson_plane.c   |  2 +-
> >>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |  7 +++---
> >>  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c|  4 +--
> >>  drivers/gpu/drm/mxsfb/mxsfb_kms.c |  2 +-
> >>  drivers/gpu/drm/nouveau/dispnv50/base507c.c   |  2 +-
> >>  drivers/gpu/drm/nouveau/dispnv50/curs507a.c   |  2 +-
> >>  drivers/gpu/drm/nouveau/dispnv50/ovly507e.c   |  2 +-
> >>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c   |  2 +-
> >>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |  2 +-
> >>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  4 +--
> >>  .../gpu/drm/selftests/test-drm_plane_helper.c | 25 +--
> >>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c|  2 +-
> >>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c|  2 +-
> >>  drivers/gpu/drm/tegra/plane.c |  5 ++--
> >>  drivers/gpu/drm/tidss/tidss_plane.c   |  3 ++-
> >>  drivers/gpu/drm/vboxvideo/vbox_mode.c |  4 +--
> >>  drivers/gpu/drm/vc4/vc4_plane.c   |  3 ++-
> >>  drivers/gpu/drm/virtio/virtgpu_plane.c|  7 --
> >>  drivers/gpu/drm/vkms/vkms_plane.c |  8 +++---
> >>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |  4 +--
> >>  drivers/gpu/drm/xlnx/zynqmp_disp.c|  2 +-
> >>  drivers/gpu/drm/zte/zx_plane.c|  4 +--
> >>  include/drm/drm_atomic_helper.h   |  7 --
> >>  38 files changed, 108 

Re: [RFC PATCH 1/3] drm: use flags instead of boolean in plane check

2020-09-16 Thread Stefan Agner
On 2020-09-11 10:50, Daniel Vetter wrote:
> On Thu, Sep 10, 2020 at 11:24:23AM +0200, Stefan Agner wrote:
>> To improve readability and make it easier to add further optional checks
>> replace the boolean parameters with a single flag bitfield as parameter
>> of drm_atomic_helper_check_plane_state.
>>
>> The regular call sites have been converted using a simple coccinelle
>> patch.
>>
>> virtual patch
> 
> Looks reasonable, but needs kerneldoc. For that either do a list in the
> kerneldoc for drm_atomic_helper_check_plane_state, or make the flags an
> enum (still setting the value for each explicitly), and then kerneldoc
> that with a link from the function to that.

The kerneldoc updates for drm_atomic_helper_check_plane_state() should
cover it mostly. I did miss update it in patch 2, so will fix that.

I thought to keep it similar to drm_atomic_helper_commit_planes() and
use #defines as well as describing the flags in the function kerneldoc.
Should I make it a list instead?

--
Stefan

> -Daniel
> 
>>
>> @@
>> expression e1, e2, e3, e4;
>> symbol true, false;
>> @@
>>
>> (
>> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, true)
>> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, DRM_PLANE_CAN_POSITION 
>> | DRM_PLANE_CAN_UPDATE_DISABLED)
>> |
>> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, false)
>> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, DRM_PLANE_CAN_POSITION)
>> |
>> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, true)
>> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 
>> DRM_PLANE_CAN_UPDATE_DISABLED)
>> |
>> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, false)
>> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 0)
>> )
>>
>> Signed-off-by: Stefan Agner 
>> ---
>> This implements what has been discussed in the thread of the patch
>> "drm: mxsfb: check framebuffer pitch":
>> https://lkml.org/lkml/2020/9/8/1342
>>
>> Before sending it out to all maintainers I wanted to get conformation
>> if the general approach is fine.
>>
>> I think in some places there should be another linebreak for the flags,
>> but this is what coccinelle gives me. Not sure if I should manually fix
>> those places...
>>
>> --
>> Stefan
>>
>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  5 ++--
>>  drivers/gpu/drm/arm/hdlcd_crtc.c  |  6 ++---
>>  drivers/gpu/drm/arm/malidp_planes.c   |  5 ++--
>>  drivers/gpu/drm/armada/armada_plane.c |  3 ++-
>>  drivers/gpu/drm/ast/ast_mode.c|  4 +--
>>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   |  4 +--
>>  drivers/gpu/drm/drm_atomic_helper.c   | 17 +++--
>>  drivers/gpu/drm/drm_plane_helper.c|  9 +++
>>  drivers/gpu/drm/drm_simple_kms_helper.c   |  2 +-
>>  drivers/gpu/drm/i915/display/intel_display.c  |  9 ---
>>  drivers/gpu/drm/i915/display/intel_sprite.c   |  6 ++---
>>  drivers/gpu/drm/imx/ipuv3-plane.c |  7 --
>>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c |  7 --
>>  drivers/gpu/drm/mediatek/mtk_drm_plane.c  |  4 +--
>>  drivers/gpu/drm/meson/meson_overlay.c |  2 +-
>>  drivers/gpu/drm/meson/meson_plane.c   |  2 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |  7 +++---
>>  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c|  4 +--
>>  drivers/gpu/drm/mxsfb/mxsfb_kms.c |  2 +-
>>  drivers/gpu/drm/nouveau/dispnv50/base507c.c   |  2 +-
>>  drivers/gpu/drm/nouveau/dispnv50/curs507a.c   |  2 +-
>>  drivers/gpu/drm/nouveau/dispnv50/ovly507e.c   |  2 +-
>>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c   |  2 +-
>>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |  2 +-
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  4 +--
>>  .../gpu/drm/selftests/test-drm_plane_helper.c | 25 +--
>>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c|  2 +-
>>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c|  2 +-
>>  drivers/gpu/drm/tegra/plane.c |  5 ++--
>>  drivers/gpu/drm/tidss/tidss_plane.c   |  3 ++-
>>  drivers/gpu/drm/vboxvideo/vbox_mode.c |  4 +--
>>  drivers/gpu/drm/vc4/vc4_plane.c   |  3 ++-
>>  drivers/gpu/drm/virtio/virtgpu_plane.c|  7 --
>>  drivers/gpu/drm/vkms/vkms_plane.c |  8 +++---
>>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |  4 +--
>>  drivers/gpu/drm/xlnx/zynqmp_disp.c|  2 +-
>>  drivers/gpu/drm/zte/zx_plane.c|  4 +--
>>  include/drm/drm_atomic_helper.h   |  7 --
>>  38 files changed, 108 insertions(+), 87 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index fd96fafec5b8..db0256ecf1a2 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -5731,8 +5731,9 @@ static int dm_plane_helper_check_state(struct 
>> drm_plane_state *state,
>>  int max_upscale = 

Re: [RFC PATCH 1/3] drm: use flags instead of boolean in plane check

2020-09-15 Thread Laurent Pinchart
Hi Stefan,

Thank you for the patch.

On Thu, Sep 10, 2020 at 11:24:23AM +0200, Stefan Agner wrote:
> To improve readability and make it easier to add further optional checks
> replace the boolean parameters with a single flag bitfield as parameter
> of drm_atomic_helper_check_plane_state.
> 
> The regular call sites have been converted using a simple coccinelle
> patch.
> 
> virtual patch
> 
> @@
> expression e1, e2, e3, e4;
> symbol true, false;
> @@
> 
> (
> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, true)
> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, DRM_PLANE_CAN_POSITION 
> | DRM_PLANE_CAN_UPDATE_DISABLED)
> |
> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, false)
> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, DRM_PLANE_CAN_POSITION)
> |
> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, true)
> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 
> DRM_PLANE_CAN_UPDATE_DISABLED)
> |
> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, false)
> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 0)
> )
> 
> Signed-off-by: Stefan Agner 
> ---
> This implements what has been discussed in the thread of the patch
> "drm: mxsfb: check framebuffer pitch":
> https://lkml.org/lkml/2020/9/8/1342
> 
> Before sending it out to all maintainers I wanted to get conformation
> if the general approach is fine.

I think it's good, yes. As Daniel pointed out it's just missing
documentation.

> I think in some places there should be another linebreak for the flags,
> but this is what coccinelle gives me. Not sure if I should manually fix
> those places...

A break after | would indeed be nice. Sometimes manual edit is required,
although I've seen coccinelle indenting code correctly before. I'm no
expert in that area.

>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  5 ++--
>  drivers/gpu/drm/arm/hdlcd_crtc.c  |  6 ++---
>  drivers/gpu/drm/arm/malidp_planes.c   |  5 ++--
>  drivers/gpu/drm/armada/armada_plane.c |  3 ++-
>  drivers/gpu/drm/ast/ast_mode.c|  4 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   |  4 +--
>  drivers/gpu/drm/drm_atomic_helper.c   | 17 +++--
>  drivers/gpu/drm/drm_plane_helper.c|  9 +++
>  drivers/gpu/drm/drm_simple_kms_helper.c   |  2 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |  9 ---
>  drivers/gpu/drm/i915/display/intel_sprite.c   |  6 ++---
>  drivers/gpu/drm/imx/ipuv3-plane.c |  7 --
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c |  7 --
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c  |  4 +--
>  drivers/gpu/drm/meson/meson_overlay.c |  2 +-
>  drivers/gpu/drm/meson/meson_plane.c   |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |  7 +++---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c|  4 +--
>  drivers/gpu/drm/mxsfb/mxsfb_kms.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/base507c.c   |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/curs507a.c   |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/ovly507e.c   |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c   |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |  2 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  4 +--
>  .../gpu/drm/selftests/test-drm_plane_helper.c | 25 +--
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c|  2 +-
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c|  2 +-
>  drivers/gpu/drm/tegra/plane.c |  5 ++--
>  drivers/gpu/drm/tidss/tidss_plane.c   |  3 ++-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c |  4 +--
>  drivers/gpu/drm/vc4/vc4_plane.c   |  3 ++-
>  drivers/gpu/drm/virtio/virtgpu_plane.c|  7 --
>  drivers/gpu/drm/vkms/vkms_plane.c |  8 +++---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |  4 +--
>  drivers/gpu/drm/xlnx/zynqmp_disp.c|  2 +-
>  drivers/gpu/drm/zte/zx_plane.c|  4 +--
>  include/drm/drm_atomic_helper.h   |  7 --
>  38 files changed, 108 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index fd96fafec5b8..db0256ecf1a2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5731,8 +5731,9 @@ static int dm_plane_helper_check_state(struct 
> drm_plane_state *state,
>   int max_upscale = INT_MAX;
>  
>   /* TODO: These should be checked against DC plane caps */
> - return drm_atomic_helper_check_plane_state(
> - state, new_crtc_state, max_downscale, max_upscale, true, true);
> + return drm_atomic_helper_check_plane_state(state, new_crtc_state,
> +max_downscale, max_upscale,
> +DRM_PLANE_CAN_POSITION | 
> DRM_PLANE_CAN_UPDATE_DISABLED);
> 

Re: [RFC PATCH 1/3] drm: use flags instead of boolean in plane check

2020-09-11 Thread Daniel Vetter
On Thu, Sep 10, 2020 at 11:24:23AM +0200, Stefan Agner wrote:
> To improve readability and make it easier to add further optional checks
> replace the boolean parameters with a single flag bitfield as parameter
> of drm_atomic_helper_check_plane_state.
> 
> The regular call sites have been converted using a simple coccinelle
> patch.
> 
> virtual patch

Looks reasonable, but needs kerneldoc. For that either do a list in the
kerneldoc for drm_atomic_helper_check_plane_state, or make the flags an
enum (still setting the value for each explicitly), and then kerneldoc
that with a link from the function to that.
-Daniel

> 
> @@
> expression e1, e2, e3, e4;
> symbol true, false;
> @@
> 
> (
> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, true)
> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, DRM_PLANE_CAN_POSITION 
> | DRM_PLANE_CAN_UPDATE_DISABLED)
> |
> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, false)
> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, DRM_PLANE_CAN_POSITION)
> |
> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, true)
> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 
> DRM_PLANE_CAN_UPDATE_DISABLED)
> |
> - drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, false)
> + drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 0)
> )
> 
> Signed-off-by: Stefan Agner 
> ---
> This implements what has been discussed in the thread of the patch
> "drm: mxsfb: check framebuffer pitch":
> https://lkml.org/lkml/2020/9/8/1342
> 
> Before sending it out to all maintainers I wanted to get conformation
> if the general approach is fine.
> 
> I think in some places there should be another linebreak for the flags,
> but this is what coccinelle gives me. Not sure if I should manually fix
> those places...
> 
> --
> Stefan
> 
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  5 ++--
>  drivers/gpu/drm/arm/hdlcd_crtc.c  |  6 ++---
>  drivers/gpu/drm/arm/malidp_planes.c   |  5 ++--
>  drivers/gpu/drm/armada/armada_plane.c |  3 ++-
>  drivers/gpu/drm/ast/ast_mode.c|  4 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   |  4 +--
>  drivers/gpu/drm/drm_atomic_helper.c   | 17 +++--
>  drivers/gpu/drm/drm_plane_helper.c|  9 +++
>  drivers/gpu/drm/drm_simple_kms_helper.c   |  2 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |  9 ---
>  drivers/gpu/drm/i915/display/intel_sprite.c   |  6 ++---
>  drivers/gpu/drm/imx/ipuv3-plane.c |  7 --
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c |  7 --
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c  |  4 +--
>  drivers/gpu/drm/meson/meson_overlay.c |  2 +-
>  drivers/gpu/drm/meson/meson_plane.c   |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |  7 +++---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c|  4 +--
>  drivers/gpu/drm/mxsfb/mxsfb_kms.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/base507c.c   |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/curs507a.c   |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/ovly507e.c   |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c   |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |  2 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  4 +--
>  .../gpu/drm/selftests/test-drm_plane_helper.c | 25 +--
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c|  2 +-
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c|  2 +-
>  drivers/gpu/drm/tegra/plane.c |  5 ++--
>  drivers/gpu/drm/tidss/tidss_plane.c   |  3 ++-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c |  4 +--
>  drivers/gpu/drm/vc4/vc4_plane.c   |  3 ++-
>  drivers/gpu/drm/virtio/virtgpu_plane.c|  7 --
>  drivers/gpu/drm/vkms/vkms_plane.c |  8 +++---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |  4 +--
>  drivers/gpu/drm/xlnx/zynqmp_disp.c|  2 +-
>  drivers/gpu/drm/zte/zx_plane.c|  4 +--
>  include/drm/drm_atomic_helper.h   |  7 --
>  38 files changed, 108 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index fd96fafec5b8..db0256ecf1a2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5731,8 +5731,9 @@ static int dm_plane_helper_check_state(struct 
> drm_plane_state *state,
>   int max_upscale = INT_MAX;
>  
>   /* TODO: These should be checked against DC plane caps */
> - return drm_atomic_helper_check_plane_state(
> - state, new_crtc_state, max_downscale, max_upscale, true, true);
> + return drm_atomic_helper_check_plane_state(state, new_crtc_state,
> +max_downscale, max_upscale,
> +DRM_PLANE_CAN_POSITION | 
> 

[RFC PATCH 1/3] drm: use flags instead of boolean in plane check

2020-09-10 Thread Stefan Agner
To improve readability and make it easier to add further optional checks
replace the boolean parameters with a single flag bitfield as parameter
of drm_atomic_helper_check_plane_state.

The regular call sites have been converted using a simple coccinelle
patch.

virtual patch

@@
expression e1, e2, e3, e4;
symbol true, false;
@@

(
- drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, true)
+ drm_atomic_helper_check_plane_state(e1, e2, e3, e4, DRM_PLANE_CAN_POSITION | 
DRM_PLANE_CAN_UPDATE_DISABLED)
|
- drm_atomic_helper_check_plane_state(e1, e2, e3, e4, true, false)
+ drm_atomic_helper_check_plane_state(e1, e2, e3, e4, DRM_PLANE_CAN_POSITION)
|
- drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, true)
+ drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 
DRM_PLANE_CAN_UPDATE_DISABLED)
|
- drm_atomic_helper_check_plane_state(e1, e2, e3, e4, false, false)
+ drm_atomic_helper_check_plane_state(e1, e2, e3, e4, 0)
)

Signed-off-by: Stefan Agner 
---
This implements what has been discussed in the thread of the patch
"drm: mxsfb: check framebuffer pitch":
https://lkml.org/lkml/2020/9/8/1342

Before sending it out to all maintainers I wanted to get conformation
if the general approach is fine.

I think in some places there should be another linebreak for the flags,
but this is what coccinelle gives me. Not sure if I should manually fix
those places...

--
Stefan

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  5 ++--
 drivers/gpu/drm/arm/hdlcd_crtc.c  |  6 ++---
 drivers/gpu/drm/arm/malidp_planes.c   |  5 ++--
 drivers/gpu/drm/armada/armada_plane.c |  3 ++-
 drivers/gpu/drm/ast/ast_mode.c|  4 +--
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   |  4 +--
 drivers/gpu/drm/drm_atomic_helper.c   | 17 +++--
 drivers/gpu/drm/drm_plane_helper.c|  9 +++
 drivers/gpu/drm/drm_simple_kms_helper.c   |  2 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  9 ---
 drivers/gpu/drm/i915/display/intel_sprite.c   |  6 ++---
 drivers/gpu/drm/imx/ipuv3-plane.c |  7 --
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c |  7 --
 drivers/gpu/drm/mediatek/mtk_drm_plane.c  |  4 +--
 drivers/gpu/drm/meson/meson_overlay.c |  2 +-
 drivers/gpu/drm/meson/meson_plane.c   |  2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |  7 +++---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c|  4 +--
 drivers/gpu/drm/mxsfb/mxsfb_kms.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/base507c.c   |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/curs507a.c   |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/ovly507e.c   |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c   |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_plane.c   |  2 +-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  4 +--
 .../gpu/drm/selftests/test-drm_plane_helper.c | 25 +--
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c|  2 +-
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c|  2 +-
 drivers/gpu/drm/tegra/plane.c |  5 ++--
 drivers/gpu/drm/tidss/tidss_plane.c   |  3 ++-
 drivers/gpu/drm/vboxvideo/vbox_mode.c |  4 +--
 drivers/gpu/drm/vc4/vc4_plane.c   |  3 ++-
 drivers/gpu/drm/virtio/virtgpu_plane.c|  7 --
 drivers/gpu/drm/vkms/vkms_plane.c |  8 +++---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |  4 +--
 drivers/gpu/drm/xlnx/zynqmp_disp.c|  2 +-
 drivers/gpu/drm/zte/zx_plane.c|  4 +--
 include/drm/drm_atomic_helper.h   |  7 --
 38 files changed, 108 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index fd96fafec5b8..db0256ecf1a2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5731,8 +5731,9 @@ static int dm_plane_helper_check_state(struct 
drm_plane_state *state,
int max_upscale = INT_MAX;
 
/* TODO: These should be checked against DC plane caps */
-   return drm_atomic_helper_check_plane_state(
-   state, new_crtc_state, max_downscale, max_upscale, true, true);
+   return drm_atomic_helper_check_plane_state(state, new_crtc_state,
+  max_downscale, max_upscale,
+  DRM_PLANE_CAN_POSITION | 
DRM_PLANE_CAN_UPDATE_DISABLED);
 }
 
 static int dm_plane_atomic_check(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index af67fefed38d..94c9a75c0c2d 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -247,9 +247,9 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
if (!state->fb && crtc_state->active)
return -EINVAL;
return drm_atomic_helper_check_plane_state(state, crtc_state,