mbox series

[0/2] drm: Treewide plane/crtc legacy state sweeping

Message ID 20241002182200.15363-1-ville.syrjala@linux.intel.com (mailing list archive)
Headers show
Series drm: Treewide plane/crtc legacy state sweeping | expand

Message

Ville Syrjälä Oct. 2, 2024, 6:21 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

An attempt to hide the drm_plane/crtc legacy state better.

This also highlights the fact that a lot of supposedly
atomic drivers are poking around in the legacy crtc state,
which is rather questionable. For planes we did force the
legacy state to NULL already to force drivers to behave.
But even then it seems capable of confusing people with
its high profile location directly under drm_plane.

This might end up as some kind of conflict
galore, but the alternative would involve trying
to wean the atomic drivers off one by one,
which would probably take forever. At least with
this the issue becomes visible and shouldn't be
forgotten as easily.

The cc list was getting way out of hand, so I had
to trim it a bit. Hopefully I didn't chop off too
many names...

Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Alain Volmat <alain.volmat@foss.st.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Andy Yan <andy.yan@rock-chips.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Danilo Krummrich <dakr@redhat.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: freedreno@lists.freedesktop.org
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: "Heiko Stübner" <heiko@sntech.de>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Jyri Sarha <jyri.sarha@iki.fi>
Cc: Karol Herbst <kherbst@redhat.com>
Cc: linux-amlogic@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.orga
Cc: linux-mediatek@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: "Maíra Canal" <mairacanal@riseup.net>
Cc: Marijn Suijten <marijn.suijten@somainline.org>
Cc: nouveau@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.orga
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Sandy Huang <hjc@rock-chips.com>
Cc: Sean Paul <sean@poorly.run>
Cc: spice-devel@lists.freedesktop.org
Cc: virtualization@lists.linux.dev
Cc: xen-devel@lists.xenproject.org
Cc: Xinhui Pan <Xinhui.Pan@amd.com>
Cc: Zack Rusin <zack.rusin@broadcom.com>

Ville Syrjälä (2):
  drm: Move plane->{fb,old_fb,crtc} to legacy sub-structure
  drm: Move crtc->{x,y,mode,enabled} to legacy sub-structure

 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    |  7 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 20 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c       |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c      |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c        | 35 ++++----
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c        | 35 ++++----
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c         | 37 ++++-----
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c         | 35 ++++----
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 ++--
 .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c    |  2 +-
 drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c  |  4 +-
 drivers/gpu/drm/arm/hdlcd_drv.c               |  2 +-
 drivers/gpu/drm/arm/malidp_hw.c               |  2 +-
 drivers/gpu/drm/armada/armada_crtc.c          | 12 ++-
 drivers/gpu/drm/ast/ast_dp.c                  |  8 +-
 drivers/gpu/drm/drm_atomic.c                  |  6 +-
 drivers/gpu/drm/drm_atomic_helper.c           |  8 +-
 drivers/gpu/drm/drm_client_modeset.c          | 10 +--
 drivers/gpu/drm/drm_crtc.c                    | 31 +++----
 drivers/gpu/drm/drm_crtc_helper.c             | 80 ++++++++++---------
 drivers/gpu/drm/drm_fb_helper.c               | 12 +--
 drivers/gpu/drm/drm_framebuffer.c             |  4 +-
 drivers/gpu/drm/drm_plane.c                   | 69 ++++++++--------
 drivers/gpu/drm/drm_plane_helper.c            |  6 +-
 drivers/gpu/drm/drm_vblank.c                  |  2 +-
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  4 +-
 drivers/gpu/drm/gma500/cdv_intel_display.c    |  2 +-
 drivers/gpu/drm/gma500/cdv_intel_dp.c         |  6 +-
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c       |  3 +-
 drivers/gpu/drm/gma500/cdv_intel_lvds.c       |  6 +-
 drivers/gpu/drm/gma500/gma_display.c          | 22 ++---
 drivers/gpu/drm/gma500/oaktrail_crtc.c        |  2 +-
 drivers/gpu/drm/gma500/psb_intel_display.c    |  2 +-
 drivers/gpu/drm/gma500/psb_intel_lvds.c       |  6 +-
 drivers/gpu/drm/gma500/psb_intel_sdvo.c       |  8 +-
 drivers/gpu/drm/i2c/ch7006_drv.c              |  7 +-
 drivers/gpu/drm/i2c/sil164_drv.c              |  2 +-
 .../drm/i915/display/intel_modeset_setup.c    |  4 +-
 drivers/gpu/drm/imx/lcdc/imx-lcdc.c           | 31 ++++---
 drivers/gpu/drm/mediatek/mtk_crtc.c           |  6 +-
 drivers/gpu/drm/meson/meson_overlay.c         |  2 +-
 drivers/gpu/drm/meson/meson_plane.c           |  8 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 18 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |  6 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c     | 16 ++--
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  4 +-
 drivers/gpu/drm/nouveau/dispnv04/crtc.c       | 25 +++---
 drivers/gpu/drm/nouveau/dispnv04/cursor.c     |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/dfp.c        |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/disp.c       |  4 +-
 .../gpu/drm/nouveau/dispnv04/tvmodesnv17.c    |  4 +-
 drivers/gpu/drm/nouveau/dispnv04/tvnv17.c     |  7 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c   |  6 +-
 drivers/gpu/drm/qxl/qxl_display.c             |  6 +-
 drivers/gpu/drm/radeon/atombios_crtc.c        | 28 +++----
 drivers/gpu/drm/radeon/cik.c                  | 12 +--
 drivers/gpu/drm/radeon/evergreen.c            | 16 ++--
 drivers/gpu/drm/radeon/r100.c                 | 16 ++--
 drivers/gpu/drm/radeon/r600_cs.c              |  2 +-
 drivers/gpu/drm/radeon/r600_dpm.c             |  4 +-
 drivers/gpu/drm/radeon/radeon_connectors.c    |  7 +-
 drivers/gpu/drm/radeon/radeon_cursor.c        | 29 +++----
 drivers/gpu/drm/radeon/radeon_device.c        |  2 +-
 drivers/gpu/drm/radeon/radeon_display.c       | 26 +++---
 drivers/gpu/drm/radeon/radeon_drv.c           |  2 +-
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c   | 16 ++--
 .../gpu/drm/radeon/radeon_legacy_encoders.c   |  2 +-
 drivers/gpu/drm/radeon/radeon_pm.c            |  2 +-
 drivers/gpu/drm/radeon/rs600.c                | 10 +--
 drivers/gpu/drm/radeon/rs690.c                | 22 ++---
 drivers/gpu/drm/radeon/rs780_dpm.c            |  6 +-
 drivers/gpu/drm/radeon/rv515.c                | 30 +++----
 drivers/gpu/drm/radeon/rv770.c                |  2 +-
 drivers/gpu/drm/radeon/si.c                   | 14 ++--
 .../gpu/drm/renesas/rcar-du/rcar_du_crtc.c    |  2 +-
 .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c |  2 +-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  6 +-
 drivers/gpu/drm/sti/sti_crtc.c                |  4 +-
 drivers/gpu/drm/sti/sti_cursor.c              |  2 +-
 drivers/gpu/drm/sti/sti_gdp.c                 |  2 +-
 drivers/gpu/drm/sti/sti_hqvdp.c               |  2 +-
 drivers/gpu/drm/sti/sti_tvout.c               |  6 +-
 drivers/gpu/drm/sti/sti_vid.c                 |  2 +-
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c          | 10 +--
 drivers/gpu/drm/tiny/arcpgu.c                 |  2 +-
 drivers/gpu/drm/vboxvideo/vbox_mode.c         |  2 +-
 drivers/gpu/drm/vc4/vc4_dpi.c                 |  2 +-
 drivers/gpu/drm/vc4/vc4_plane.c               |  4 +-
 drivers/gpu/drm/virtio/virtgpu_display.c      |  4 +-
 drivers/gpu/drm/vkms/vkms_composer.c          |  4 +-
 drivers/gpu/drm/vkms/vkms_crtc.c              |  2 +-
 drivers/gpu/drm/vkms/vkms_writeback.c         |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  8 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c           | 18 +++--
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c          |  9 ++-
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c          |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c          |  2 +-
 drivers/gpu/drm/xen/xen_drm_front_kms.c       |  2 +-
 include/drm/drm_crtc.h                        | 75 ++++++++---------
 include/drm/drm_plane.h                       | 52 ++++++------
 100 files changed, 599 insertions(+), 547 deletions(-)

Comments

Louis Chauvet Oct. 3, 2024, 12:38 p.m. UTC | #1
Le 02/10/24 - 21:22, Ville Syrjala a écrit :
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Atomic drivers shouldn't be using the legacy state stored
> directly under drm_crtc. Move that junk into a 'legacy' sub
> structure to highlight the offenders, of which there are
> quite a few unfortunately.

Hi,

Do we need to do something particular in an atomic driver except using
state content?

I proposed some modifications for VKMS bellow. If you think this is good,
I can send a patch to avoid being an offender :-) I just tested it, and it
seems to work.

> I'm hoping we could get all these fixed and then declare
> the legacy state off limits for atomic drivers (which is
> what did long ago for plane->fb/etc). And maybe eventually
> turn crtc->legacy into a pointer and only allocate it on
> legacy drivers.
> 
> TODO: hwmode should probably go there too but it probably
>       needs a closer look, maybe other stuff too...

[...]

> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> index 57a5769fc994..a7f8b1da6e85 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -187,7 +187,7 @@ static void blend(struct vkms_writeback_job *wb,
>  
>  	const struct pixel_argb_u16 background_color = { .a = 0xffff };
>  
> -	size_t crtc_y_limit = crtc_state->base.crtc->mode.vdisplay;
> +	size_t crtc_y_limit = crtc_state->base.crtc->legacy.mode.vdisplay;

	size_t crtc_y_limit = crtc_state->base.mode.vdisplay;

>  	/*
>  	 * The planes are composed line-by-line to avoid heavy memory usage. It is a necessary
> @@ -270,7 +270,7 @@ static int compose_active_planes(struct vkms_writeback_job *active_wb,
>  	if (WARN_ON(check_format_funcs(crtc_state, active_wb)))
>  		return -EINVAL;
>  
> -	line_width = crtc_state->base.crtc->mode.hdisplay;
> +	line_width = crtc_state->base.crtc->legacy.mode.hdisplay;

	line_width = crtc_state->base.mode.hdisplay;

>  	stage_buffer.n_pixels = line_width;
>  	output_buffer.n_pixels = line_width;
>  
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index a40295c18b48..780681ea77e4 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -64,7 +64,7 @@ static int vkms_enable_vblank(struct drm_crtc *crtc)
>  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>  	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
>  
> -	drm_calc_timestamping_constants(crtc, &crtc->mode);
> +	drm_calc_timestamping_constants(crtc, &crtc->legacy.mode);

	drm_calc_timestamping_constants(crtc, &crtc->state->mode);

>  	hrtimer_init(&out->vblank_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>  	out->vblank_hrtimer.function = &vkms_vblank_simulate;
> diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
> index bc724cbd5e3a..27164cddb94d 100644
> --- a/drivers/gpu/drm/vkms/vkms_writeback.c
> +++ b/drivers/gpu/drm/vkms/vkms_writeback.c
> @@ -131,8 +131,8 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
>  	struct drm_connector_state *conn_state = wb_conn->base.state;
>  	struct vkms_crtc_state *crtc_state = output->composer_state;
>  	struct drm_framebuffer *fb = connector_state->writeback_job->fb;
> -	u16 crtc_height = crtc_state->base.crtc->mode.vdisplay;
> -	u16 crtc_width = crtc_state->base.crtc->mode.hdisplay;
> +	u16 crtc_height = crtc_state->base.crtc->legacy.mode.vdisplay;
> +	u16 crtc_width = crtc_state->base.crtc->legacy.mode.hdisplay;

	u16 crtc_height = crtc_state->base.mode.vdisplay;
	u16 crtc_width = crtc_state->base.mode.hdisplay;

>  	struct vkms_writeback_job *active_wb;
>  	struct vkms_frame_info *wb_frame_info;
>  	u32 wb_format = fb->format->format;

[...]
Ville Syrjälä Oct. 3, 2024, 1:46 p.m. UTC | #2
On Thu, Oct 03, 2024 at 02:38:35PM +0200, Louis Chauvet wrote:
> Le 02/10/24 - 21:22, Ville Syrjala a écrit :
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Atomic drivers shouldn't be using the legacy state stored
> > directly under drm_crtc. Move that junk into a 'legacy' sub
> > structure to highlight the offenders, of which there are
> > quite a few unfortunately.
> 
> Hi,
> 
> Do we need to do something particular in an atomic driver except using
> state content?
> 
> I proposed some modifications for VKMS bellow. If you think this is good,
> I can send a patch to avoid being an offender :-) I just tested it, and it
> seems to work.
> 
> > I'm hoping we could get all these fixed and then declare
> > the legacy state off limits for atomic drivers (which is
> > what did long ago for plane->fb/etc). And maybe eventually
> > turn crtc->legacy into a pointer and only allocate it on
> > legacy drivers.
> > 
> > TODO: hwmode should probably go there too but it probably
> >       needs a closer look, maybe other stuff too...
> 
> [...]
> 
> > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
> > index 57a5769fc994..a7f8b1da6e85 100644
> > --- a/drivers/gpu/drm/vkms/vkms_composer.c
> > +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> > @@ -187,7 +187,7 @@ static void blend(struct vkms_writeback_job *wb,
> >  
> >  	const struct pixel_argb_u16 background_color = { .a = 0xffff };
> >  
> > -	size_t crtc_y_limit = crtc_state->base.crtc->mode.vdisplay;
> > +	size_t crtc_y_limit = crtc_state->base.crtc->legacy.mode.vdisplay;
> 
> 	size_t crtc_y_limit = crtc_state->base.mode.vdisplay;
> 
> >  	/*
> >  	 * The planes are composed line-by-line to avoid heavy memory usage. It is a necessary
> > @@ -270,7 +270,7 @@ static int compose_active_planes(struct vkms_writeback_job *active_wb,
> >  	if (WARN_ON(check_format_funcs(crtc_state, active_wb)))
> >  		return -EINVAL;
> >  
> > -	line_width = crtc_state->base.crtc->mode.hdisplay;
> > +	line_width = crtc_state->base.crtc->legacy.mode.hdisplay;
> 
> 	line_width = crtc_state->base.mode.hdisplay;
> 
> >  	stage_buffer.n_pixels = line_width;
> >  	output_buffer.n_pixels = line_width;
> >  
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > index a40295c18b48..780681ea77e4 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -64,7 +64,7 @@ static int vkms_enable_vblank(struct drm_crtc *crtc)
> >  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> >  	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> >  
> > -	drm_calc_timestamping_constants(crtc, &crtc->mode);
> > +	drm_calc_timestamping_constants(crtc, &crtc->legacy.mode);
> 
> 	drm_calc_timestamping_constants(crtc, &crtc->state->mode);

This one doesn't look safe. You want to call that during your atomic
commit already.

The rest look reasonable.

> 
> >  	hrtimer_init(&out->vblank_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> >  	out->vblank_hrtimer.function = &vkms_vblank_simulate;
> > diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
> > index bc724cbd5e3a..27164cddb94d 100644
> > --- a/drivers/gpu/drm/vkms/vkms_writeback.c
> > +++ b/drivers/gpu/drm/vkms/vkms_writeback.c
> > @@ -131,8 +131,8 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
> >  	struct drm_connector_state *conn_state = wb_conn->base.state;
> >  	struct vkms_crtc_state *crtc_state = output->composer_state;
> >  	struct drm_framebuffer *fb = connector_state->writeback_job->fb;
> > -	u16 crtc_height = crtc_state->base.crtc->mode.vdisplay;
> > -	u16 crtc_width = crtc_state->base.crtc->mode.hdisplay;
> > +	u16 crtc_height = crtc_state->base.crtc->legacy.mode.vdisplay;
> > +	u16 crtc_width = crtc_state->base.crtc->legacy.mode.hdisplay;
> 
> 	u16 crtc_height = crtc_state->base.mode.vdisplay;
> 	u16 crtc_width = crtc_state->base.mode.hdisplay;
> 
> >  	struct vkms_writeback_job *active_wb;
> >  	struct vkms_frame_info *wb_frame_info;
> >  	u32 wb_format = fb->format->format;
> 
> [...]
> 
> -- 
> Louis Chauvet, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Louis Chauvet Oct. 3, 2024, 3:07 p.m. UTC | #3
> > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > index a40295c18b48..780681ea77e4 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > @@ -64,7 +64,7 @@ static int vkms_enable_vblank(struct drm_crtc *crtc)
> > >  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> > >  	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> > >  
> > > -	drm_calc_timestamping_constants(crtc, &crtc->mode);
> > > +	drm_calc_timestamping_constants(crtc, &crtc->legacy.mode);
> > 
> > 	drm_calc_timestamping_constants(crtc, &crtc->state->mode);
> 
> This one doesn't look safe. You want to call that during your atomic
> commit already.
> 

This was already not safe with the previous implementation? Or it is only 
unsafe because now I use state->mode instead of legacy.mode?

After inspecting the code, I think I don't need to call it as:

In `vkms_atomic_commit_tail` (used in 
`@vkms_mode_config_helpers.atomic_commit_tail`), we call 
`drm_atomic_helper_commit_modeset_disables`, which call 
`drm_atomic_helper_calc_timestamping_constants` which call 
`drm_calc_timestamping_constants` for every CRTC.

I tested kms_vblank, all of them are SUCCESS/SKIP, do you know other tests 
that can trigger bugs?
Ville Syrjälä Oct. 3, 2024, 3:29 p.m. UTC | #4
On Thu, Oct 03, 2024 at 05:07:35PM +0200, Louis Chauvet wrote:
> 
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > index a40295c18b48..780681ea77e4 100644
> > > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > @@ -64,7 +64,7 @@ static int vkms_enable_vblank(struct drm_crtc *crtc)
> > > >  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> > > >  	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> > > >  
> > > > -	drm_calc_timestamping_constants(crtc, &crtc->mode);
> > > > +	drm_calc_timestamping_constants(crtc, &crtc->legacy.mode);
> > > 
> > > 	drm_calc_timestamping_constants(crtc, &crtc->state->mode);
> > 
> > This one doesn't look safe. You want to call that during your atomic
> > commit already.
> > 
> 
> This was already not safe with the previous implementation? Or it is only 
> unsafe because now I use state->mode instead of legacy.mode?

Yeah, if you want to look at obj->state then you need the corresponding
lock.

obj->state is also not necessarily the correct state you want because
a parallel commit could have already swapped in a new state but the
hardware is still on the old state.

Basically 99.9% of code should never even look at obj->state, and
instead should always use the for_each_new_<obj>_in_state()
and drm_atomic_get_new_<obj>_state() stuff. Currently that is a
pipe dream though because a lot of drivers haven't been fixed to
do things properly. If we ever manage to fix everything then we
could remove the stall hacks from drm_atomic_helper_swap_state()
and allow a commit pipeline of arbitrary length.

> 
> After inspecting the code, I think I don't need to call it as:
> 
> In `vkms_atomic_commit_tail` (used in 
> `@vkms_mode_config_helpers.atomic_commit_tail`), we call 
> `drm_atomic_helper_commit_modeset_disables`, which call 
> `drm_atomic_helper_calc_timestamping_constants` which call 
> `drm_calc_timestamping_constants` for every CRTC.

Slightly odd place for it, but I think that's just because it was
originally part of drm_atomic_helper_update_legacy_modeset_state()
and I didn't bother looking for a better home for it when I split
it out. But seems like it should work fine as is.

> 
> I tested kms_vblank, all of them are SUCCESS/SKIP, do you know other tests 
> that can trigger bugs?

You would explicitly have to race commits against vblank_enable.
Could of course sprinkle sleep()s around to widen the race window
if you're really keen to hit it.
Louis Chauvet Oct. 3, 2024, 3:45 p.m. UTC | #5
Le 03/10/24 - 18:29, Ville Syrjälä a écrit :
> On Thu, Oct 03, 2024 at 05:07:35PM +0200, Louis Chauvet wrote:
> > 
> > > > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > > index a40295c18b48..780681ea77e4 100644
> > > > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > > @@ -64,7 +64,7 @@ static int vkms_enable_vblank(struct drm_crtc *crtc)
> > > > >  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> > > > >  	struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
> > > > >  
> > > > > -	drm_calc_timestamping_constants(crtc, &crtc->mode);
> > > > > +	drm_calc_timestamping_constants(crtc, &crtc->legacy.mode);
> > > > 
> > > > 	drm_calc_timestamping_constants(crtc, &crtc->state->mode);
> > > 
> > > This one doesn't look safe. You want to call that during your atomic
> > > commit already.
> > > 
> > 
> > This was already not safe with the previous implementation? Or it is only 
> > unsafe because now I use state->mode instead of legacy.mode?
> 
> Yeah, if you want to look at obj->state then you need the corresponding
> lock.
> 
> obj->state is also not necessarily the correct state you want because
> a parallel commit could have already swapped in a new state but the
> hardware is still on the old state.
> 
> Basically 99.9% of code should never even look at obj->state, and
> instead should always use the for_each_new_<obj>_in_state()
> and drm_atomic_get_new_<obj>_state() stuff. Currently that is a
> pipe dream though because a lot of drivers haven't been fixed to
> do things properly. If we ever manage to fix everything then we
> could remove the stall hacks from drm_atomic_helper_swap_state()
> and allow a commit pipeline of arbitrary length.
>
> > 
> > After inspecting the code, I think I don't need to call it as:
> > 
> > In `vkms_atomic_commit_tail` (used in 
> > `@vkms_mode_config_helpers.atomic_commit_tail`), we call 
> > `drm_atomic_helper_commit_modeset_disables`, which call 
> > `drm_atomic_helper_calc_timestamping_constants` which call 
> > `drm_calc_timestamping_constants` for every CRTC.
> 
> Slightly odd place for it, but I think that's just because it was
> originally part of drm_atomic_helper_update_legacy_modeset_state()
> and I didn't bother looking for a better home for it when I split
> it out. But seems like it should work fine as is.

I just send a patch for this! Thanks for your help!

[1]:https://lore.kernel.org/all/20241003-remove-legacy-v1-1-0b7db1f1a1a6@bootlin.com/
 
> > 
> > I tested kms_vblank, all of them are SUCCESS/SKIP, do you know other tests 
> > that can trigger bugs?
> 
> You would explicitly have to race commits against vblank_enable.
> Could of course sprinkle sleep()s around to widen the race window
> if you're really keen to hit it.
> 
> -- 
> Ville Syrjälä
> Intel
Ville Syrjälä Oct. 25, 2024, 7:46 a.m. UTC | #6
On Wed, Oct 02, 2024 at 09:21:58PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> An attempt to hide the drm_plane/crtc legacy state better.
> 
> This also highlights the fact that a lot of supposedly
> atomic drivers are poking around in the legacy crtc state,
> which is rather questionable. For planes we did force the
> legacy state to NULL already to force drivers to behave.
> But even then it seems capable of confusing people with
> its high profile location directly under drm_plane.
> 
> This might end up as some kind of conflict
> galore, but the alternative would involve trying
> to wean the atomic drivers off one by one,
> which would probably take forever. At least with
> this the issue becomes visible and shouldn't be
> forgotten as easily.

Ping, anyone have thoughts on this? I'd like to get something
like this in at some point to make the legacy state (ab)users
easily visible...

> 
> The cc list was getting way out of hand, so I had
> to trim it a bit. Hopefully I didn't chop off too
> many names...
> 
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Alain Volmat <alain.volmat@foss.st.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Andy Yan <andy.yan@rock-chips.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Danilo Krummrich <dakr@redhat.com>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Cc: freedreno@lists.freedesktop.org
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: "Heiko Stübner" <heiko@sntech.de>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Jyri Sarha <jyri.sarha@iki.fi>
> Cc: Karol Herbst <kherbst@redhat.com>
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: linux-arm-msm@vger.kernel.orga
> Cc: linux-mediatek@lists.infradead.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: "Maíra Canal" <mairacanal@riseup.net>
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: nouveau@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.orga
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Sandy Huang <hjc@rock-chips.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: spice-devel@lists.freedesktop.org
> Cc: virtualization@lists.linux.dev
> Cc: xen-devel@lists.xenproject.org
> Cc: Xinhui Pan <Xinhui.Pan@amd.com>
> Cc: Zack Rusin <zack.rusin@broadcom.com>
> 
> Ville Syrjälä (2):
>   drm: Move plane->{fb,old_fb,crtc} to legacy sub-structure
>   drm: Move crtc->{x,y,mode,enabled} to legacy sub-structure
> 
>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    |  7 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 20 ++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c       |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c      |  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c        | 35 ++++----
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c        | 35 ++++----
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c         | 37 ++++-----
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c         | 35 ++++----
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 ++--
>  .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c    |  2 +-
>  drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c  |  4 +-
>  drivers/gpu/drm/arm/hdlcd_drv.c               |  2 +-
>  drivers/gpu/drm/arm/malidp_hw.c               |  2 +-
>  drivers/gpu/drm/armada/armada_crtc.c          | 12 ++-
>  drivers/gpu/drm/ast/ast_dp.c                  |  8 +-
>  drivers/gpu/drm/drm_atomic.c                  |  6 +-
>  drivers/gpu/drm/drm_atomic_helper.c           |  8 +-
>  drivers/gpu/drm/drm_client_modeset.c          | 10 +--
>  drivers/gpu/drm/drm_crtc.c                    | 31 +++----
>  drivers/gpu/drm/drm_crtc_helper.c             | 80 ++++++++++---------
>  drivers/gpu/drm/drm_fb_helper.c               | 12 +--
>  drivers/gpu/drm/drm_framebuffer.c             |  4 +-
>  drivers/gpu/drm/drm_plane.c                   | 69 ++++++++--------
>  drivers/gpu/drm/drm_plane_helper.c            |  6 +-
>  drivers/gpu/drm/drm_vblank.c                  |  2 +-
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  4 +-
>  drivers/gpu/drm/gma500/cdv_intel_display.c    |  2 +-
>  drivers/gpu/drm/gma500/cdv_intel_dp.c         |  6 +-
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c       |  3 +-
>  drivers/gpu/drm/gma500/cdv_intel_lvds.c       |  6 +-
>  drivers/gpu/drm/gma500/gma_display.c          | 22 ++---
>  drivers/gpu/drm/gma500/oaktrail_crtc.c        |  2 +-
>  drivers/gpu/drm/gma500/psb_intel_display.c    |  2 +-
>  drivers/gpu/drm/gma500/psb_intel_lvds.c       |  6 +-
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c       |  8 +-
>  drivers/gpu/drm/i2c/ch7006_drv.c              |  7 +-
>  drivers/gpu/drm/i2c/sil164_drv.c              |  2 +-
>  .../drm/i915/display/intel_modeset_setup.c    |  4 +-
>  drivers/gpu/drm/imx/lcdc/imx-lcdc.c           | 31 ++++---
>  drivers/gpu/drm/mediatek/mtk_crtc.c           |  6 +-
>  drivers/gpu/drm/meson/meson_overlay.c         |  2 +-
>  drivers/gpu/drm/meson/meson_plane.c           |  8 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 18 +++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |  6 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c     | 16 ++--
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c       | 25 +++---
>  drivers/gpu/drm/nouveau/dispnv04/cursor.c     |  2 +-
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c        |  2 +-
>  drivers/gpu/drm/nouveau/dispnv04/disp.c       |  4 +-
>  .../gpu/drm/nouveau/dispnv04/tvmodesnv17.c    |  4 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c     |  7 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  6 +-
>  drivers/gpu/drm/qxl/qxl_display.c             |  6 +-
>  drivers/gpu/drm/radeon/atombios_crtc.c        | 28 +++----
>  drivers/gpu/drm/radeon/cik.c                  | 12 +--
>  drivers/gpu/drm/radeon/evergreen.c            | 16 ++--
>  drivers/gpu/drm/radeon/r100.c                 | 16 ++--
>  drivers/gpu/drm/radeon/r600_cs.c              |  2 +-
>  drivers/gpu/drm/radeon/r600_dpm.c             |  4 +-
>  drivers/gpu/drm/radeon/radeon_connectors.c    |  7 +-
>  drivers/gpu/drm/radeon/radeon_cursor.c        | 29 +++----
>  drivers/gpu/drm/radeon/radeon_device.c        |  2 +-
>  drivers/gpu/drm/radeon/radeon_display.c       | 26 +++---
>  drivers/gpu/drm/radeon/radeon_drv.c           |  2 +-
>  drivers/gpu/drm/radeon/radeon_legacy_crtc.c   | 16 ++--
>  .../gpu/drm/radeon/radeon_legacy_encoders.c   |  2 +-
>  drivers/gpu/drm/radeon/radeon_pm.c            |  2 +-
>  drivers/gpu/drm/radeon/rs600.c                | 10 +--
>  drivers/gpu/drm/radeon/rs690.c                | 22 ++---
>  drivers/gpu/drm/radeon/rs780_dpm.c            |  6 +-
>  drivers/gpu/drm/radeon/rv515.c                | 30 +++----
>  drivers/gpu/drm/radeon/rv770.c                |  2 +-
>  drivers/gpu/drm/radeon/si.c                   | 14 ++--
>  .../gpu/drm/renesas/rcar-du/rcar_du_crtc.c    |  2 +-
>  .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c |  2 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  6 +-
>  drivers/gpu/drm/sti/sti_crtc.c                |  4 +-
>  drivers/gpu/drm/sti/sti_cursor.c              |  2 +-
>  drivers/gpu/drm/sti/sti_gdp.c                 |  2 +-
>  drivers/gpu/drm/sti/sti_hqvdp.c               |  2 +-
>  drivers/gpu/drm/sti/sti_tvout.c               |  6 +-
>  drivers/gpu/drm/sti/sti_vid.c                 |  2 +-
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c          | 10 +--
>  drivers/gpu/drm/tiny/arcpgu.c                 |  2 +-
>  drivers/gpu/drm/vboxvideo/vbox_mode.c         |  2 +-
>  drivers/gpu/drm/vc4/vc4_dpi.c                 |  2 +-
>  drivers/gpu/drm/vc4/vc4_plane.c               |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_display.c      |  4 +-
>  drivers/gpu/drm/vkms/vkms_composer.c          |  4 +-
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  2 +-
>  drivers/gpu/drm/vkms/vkms_writeback.c         |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  8 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c           | 18 +++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c          |  9 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c          |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c          |  2 +-
>  drivers/gpu/drm/xen/xen_drm_front_kms.c       |  2 +-
>  include/drm/drm_crtc.h                        | 75 ++++++++---------
>  include/drm/drm_plane.h                       | 52 ++++++------
>  100 files changed, 599 insertions(+), 547 deletions(-)
> 
> -- 
> 2.45.2
Dmitry Baryshkov Oct. 25, 2024, 9:54 a.m. UTC | #7
On Fri, 25 Oct 2024 at 10:46, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Wed, Oct 02, 2024 at 09:21:58PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > An attempt to hide the drm_plane/crtc legacy state better.
> >
> > This also highlights the fact that a lot of supposedly
> > atomic drivers are poking around in the legacy crtc state,
> > which is rather questionable. For planes we did force the
> > legacy state to NULL already to force drivers to behave.
> > But even then it seems capable of confusing people with
> > its high profile location directly under drm_plane.
> >
> > This might end up as some kind of conflict
> > galore, but the alternative would involve trying
> > to wean the atomic drivers off one by one,
> > which would probably take forever. At least with
> > this the issue becomes visible and shouldn't be
> > forgotten as easily.
>
> Ping, anyone have thoughts on this? I'd like to get something
> like this in at some point to make the legacy state (ab)users
> easily visible...

I think that's a good idea. I hope to find a time slot and check the
(ab)using of legacy state in drm/msm driver.

>
> >
> > The cc list was getting way out of hand, so I had
> > to trim it a bit. Hopefully I didn't chop off too
> > many names...
Jani Nikula Oct. 25, 2024, 9:59 a.m. UTC | #8
On Fri, 25 Oct 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Oct 02, 2024 at 09:21:58PM +0300, Ville Syrjala wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> 
>> An attempt to hide the drm_plane/crtc legacy state better.
>> 
>> This also highlights the fact that a lot of supposedly
>> atomic drivers are poking around in the legacy crtc state,
>> which is rather questionable. For planes we did force the
>> legacy state to NULL already to force drivers to behave.
>> But even then it seems capable of confusing people with
>> its high profile location directly under drm_plane.
>> 
>> This might end up as some kind of conflict
>> galore, but the alternative would involve trying
>> to wean the atomic drivers off one by one,
>> which would probably take forever. At least with
>> this the issue becomes visible and shouldn't be
>> forgotten as easily.
>
> Ping, anyone have thoughts on this? I'd like to get something
> like this in at some point to make the legacy state (ab)users
> easily visible...

On the approach,

Acked-by: Jani Nikula <jani.nikula@intel.com>

with or without converting legacy into a pointer, up to you.

>
>> 
>> The cc list was getting way out of hand, so I had
>> to trim it a bit. Hopefully I didn't chop off too
>> many names...
>> 
>> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Cc: Alain Volmat <alain.volmat@foss.st.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Alexey Brodkin <abrodkin@synopsys.com>
>> Cc: amd-gfx@lists.freedesktop.org
>> Cc: Andy Yan <andy.yan@rock-chips.com>
>> Cc: "Christian König" <christian.koenig@amd.com>
>> Cc: Danilo Krummrich <dakr@redhat.com>
>> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Cc: freedreno@lists.freedesktop.org
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: "Heiko Stübner" <heiko@sntech.de>
>> Cc: Inki Dae <inki.dae@samsung.com>
>> Cc: Jyri Sarha <jyri.sarha@iki.fi>
>> Cc: Karol Herbst <kherbst@redhat.com>
>> Cc: linux-amlogic@lists.infradead.org
>> Cc: linux-arm-msm@vger.kernel.org
>> Cc: linux-arm-msm@vger.kernel.orga
>> Cc: linux-mediatek@lists.infradead.org
>> Cc: linux-renesas-soc@vger.kernel.org
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Cc: "Maíra Canal" <mairacanal@riseup.net>
>> Cc: Marijn Suijten <marijn.suijten@somainline.org>
>> Cc: nouveau@lists.freedesktop.org
>> Cc: nouveau@lists.freedesktop.orga
>> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
>> Cc: Rob Clark <robdclark@gmail.com>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: Sandy Huang <hjc@rock-chips.com>
>> Cc: Sean Paul <sean@poorly.run>
>> Cc: spice-devel@lists.freedesktop.org
>> Cc: virtualization@lists.linux.dev
>> Cc: xen-devel@lists.xenproject.org
>> Cc: Xinhui Pan <Xinhui.Pan@amd.com>
>> Cc: Zack Rusin <zack.rusin@broadcom.com>
>> 
>> Ville Syrjälä (2):
>>   drm: Move plane->{fb,old_fb,crtc} to legacy sub-structure
>>   drm: Move crtc->{x,y,mode,enabled} to legacy sub-structure
>> 
>>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    |  7 +-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 20 ++---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_pll.c       |  2 +-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c      |  2 +-
>>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c        | 35 ++++----
>>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c        | 35 ++++----
>>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c         | 37 ++++-----
>>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c         | 35 ++++----
>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 ++--
>>  .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c    |  2 +-
>>  drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c  |  4 +-
>>  drivers/gpu/drm/arm/hdlcd_drv.c               |  2 +-
>>  drivers/gpu/drm/arm/malidp_hw.c               |  2 +-
>>  drivers/gpu/drm/armada/armada_crtc.c          | 12 ++-
>>  drivers/gpu/drm/ast/ast_dp.c                  |  8 +-
>>  drivers/gpu/drm/drm_atomic.c                  |  6 +-
>>  drivers/gpu/drm/drm_atomic_helper.c           |  8 +-
>>  drivers/gpu/drm/drm_client_modeset.c          | 10 +--
>>  drivers/gpu/drm/drm_crtc.c                    | 31 +++----
>>  drivers/gpu/drm/drm_crtc_helper.c             | 80 ++++++++++---------
>>  drivers/gpu/drm/drm_fb_helper.c               | 12 +--
>>  drivers/gpu/drm/drm_framebuffer.c             |  4 +-
>>  drivers/gpu/drm/drm_plane.c                   | 69 ++++++++--------
>>  drivers/gpu/drm/drm_plane_helper.c            |  6 +-
>>  drivers/gpu/drm/drm_vblank.c                  |  2 +-
>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  4 +-
>>  drivers/gpu/drm/gma500/cdv_intel_display.c    |  2 +-
>>  drivers/gpu/drm/gma500/cdv_intel_dp.c         |  6 +-
>>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c       |  3 +-
>>  drivers/gpu/drm/gma500/cdv_intel_lvds.c       |  6 +-
>>  drivers/gpu/drm/gma500/gma_display.c          | 22 ++---
>>  drivers/gpu/drm/gma500/oaktrail_crtc.c        |  2 +-
>>  drivers/gpu/drm/gma500/psb_intel_display.c    |  2 +-
>>  drivers/gpu/drm/gma500/psb_intel_lvds.c       |  6 +-
>>  drivers/gpu/drm/gma500/psb_intel_sdvo.c       |  8 +-
>>  drivers/gpu/drm/i2c/ch7006_drv.c              |  7 +-
>>  drivers/gpu/drm/i2c/sil164_drv.c              |  2 +-
>>  .../drm/i915/display/intel_modeset_setup.c    |  4 +-
>>  drivers/gpu/drm/imx/lcdc/imx-lcdc.c           | 31 ++++---
>>  drivers/gpu/drm/mediatek/mtk_crtc.c           |  6 +-
>>  drivers/gpu/drm/meson/meson_overlay.c         |  2 +-
>>  drivers/gpu/drm/meson/meson_plane.c           |  8 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 18 +++--
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |  6 +-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c     | 16 ++--
>>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  4 +-
>>  drivers/gpu/drm/nouveau/dispnv04/crtc.c       | 25 +++---
>>  drivers/gpu/drm/nouveau/dispnv04/cursor.c     |  2 +-
>>  drivers/gpu/drm/nouveau/dispnv04/dfp.c        |  2 +-
>>  drivers/gpu/drm/nouveau/dispnv04/disp.c       |  4 +-
>>  .../gpu/drm/nouveau/dispnv04/tvmodesnv17.c    |  4 +-
>>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c     |  7 +-
>>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  6 +-
>>  drivers/gpu/drm/qxl/qxl_display.c             |  6 +-
>>  drivers/gpu/drm/radeon/atombios_crtc.c        | 28 +++----
>>  drivers/gpu/drm/radeon/cik.c                  | 12 +--
>>  drivers/gpu/drm/radeon/evergreen.c            | 16 ++--
>>  drivers/gpu/drm/radeon/r100.c                 | 16 ++--
>>  drivers/gpu/drm/radeon/r600_cs.c              |  2 +-
>>  drivers/gpu/drm/radeon/r600_dpm.c             |  4 +-
>>  drivers/gpu/drm/radeon/radeon_connectors.c    |  7 +-
>>  drivers/gpu/drm/radeon/radeon_cursor.c        | 29 +++----
>>  drivers/gpu/drm/radeon/radeon_device.c        |  2 +-
>>  drivers/gpu/drm/radeon/radeon_display.c       | 26 +++---
>>  drivers/gpu/drm/radeon/radeon_drv.c           |  2 +-
>>  drivers/gpu/drm/radeon/radeon_legacy_crtc.c   | 16 ++--
>>  .../gpu/drm/radeon/radeon_legacy_encoders.c   |  2 +-
>>  drivers/gpu/drm/radeon/radeon_pm.c            |  2 +-
>>  drivers/gpu/drm/radeon/rs600.c                | 10 +--
>>  drivers/gpu/drm/radeon/rs690.c                | 22 ++---
>>  drivers/gpu/drm/radeon/rs780_dpm.c            |  6 +-
>>  drivers/gpu/drm/radeon/rv515.c                | 30 +++----
>>  drivers/gpu/drm/radeon/rv770.c                |  2 +-
>>  drivers/gpu/drm/radeon/si.c                   | 14 ++--
>>  .../gpu/drm/renesas/rcar-du/rcar_du_crtc.c    |  2 +-
>>  .../gpu/drm/renesas/shmobile/shmob_drm_crtc.c |  2 +-
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  6 +-
>>  drivers/gpu/drm/sti/sti_crtc.c                |  4 +-
>>  drivers/gpu/drm/sti/sti_cursor.c              |  2 +-
>>  drivers/gpu/drm/sti/sti_gdp.c                 |  2 +-
>>  drivers/gpu/drm/sti/sti_hqvdp.c               |  2 +-
>>  drivers/gpu/drm/sti/sti_tvout.c               |  6 +-
>>  drivers/gpu/drm/sti/sti_vid.c                 |  2 +-
>>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c          | 10 +--
>>  drivers/gpu/drm/tiny/arcpgu.c                 |  2 +-
>>  drivers/gpu/drm/vboxvideo/vbox_mode.c         |  2 +-
>>  drivers/gpu/drm/vc4/vc4_dpi.c                 |  2 +-
>>  drivers/gpu/drm/vc4/vc4_plane.c               |  4 +-
>>  drivers/gpu/drm/virtio/virtgpu_display.c      |  4 +-
>>  drivers/gpu/drm/vkms/vkms_composer.c          |  4 +-
>>  drivers/gpu/drm/vkms/vkms_crtc.c              |  2 +-
>>  drivers/gpu/drm/vkms/vkms_writeback.c         |  4 +-
>>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  8 +-
>>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c           | 18 +++--
>>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c          |  9 ++-
>>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c          |  4 +-
>>  drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c          |  2 +-
>>  drivers/gpu/drm/xen/xen_drm_front_kms.c       |  2 +-
>>  include/drm/drm_crtc.h                        | 75 ++++++++---------
>>  include/drm/drm_plane.h                       | 52 ++++++------
>>  100 files changed, 599 insertions(+), 547 deletions(-)
>> 
>> -- 
>> 2.45.2