mbox series

[00/22] drm: Review of mode copies

Message ID 20220218100403.7028-1-ville.syrjala@linux.intel.com (mailing list archive)
Headers show
Series drm: Review of mode copies | expand

Message

Ville Syrjälä Feb. 18, 2022, 10:03 a.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I might be taking this a bit too far, but the lack of
consistency in our methods to copy drm_display_mode
structs around is bugging me.

The main worry is the embedded list head, which if
clobbered could lead to list corruption. I'd also
prefer to make sure even the valid list heads don't
propagate between copies since that makes no sense.

While going through some of the code I also spotted
some very weird on stack copies being made for no
reason at all. I elimininated a few of them here,
but there could certainly be more lurking in the
shadows.

Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Alain Volmat <alain.volmat@foss.st.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
Cc: Chen Feng <puck.chen@hisilicon.com>
Cc: Chun-Kuang Hu <chunkuang.hu@kernel.org>
Cc: Emma Anholt <emma@anholt.net>
Cc: freedreno@lists.freedesktop.org
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: "Heiko Stübner" <heiko@sntech.de>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jyri Sarha <jyri.sarha@iki.fi>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Nikola Cornij <nikola.cornij@amd.com>
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Sandy Huang <hjc@rock-chips.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Tian Tao <tiantao6@hisilicon.com>
Cc: Tomi Valkeinen <tomba@kernel.org>
Cc: Xinliang Liu <xinliang.liu@linaro.org>
Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>

Ville Syrjälä (22):
  drm: Add drm_mode_init()
  drm/amdgpu: Remove pointless on stack mode copies
  drm/amdgpu: Use drm_mode_init() for on-stack modes
  drm/amdgpu: Use drm_mode_copy()
  drm/radeon: Use drm_mode_copy()
  drm/bridge: Use drm_mode_copy()
  drm/gma500: Use drm_mode_copy()
  drm/hisilicon: Use drm_mode_init() for on-stack modes
  drm/imx: Use drm_mode_duplicate()
  drm/msm: Nuke weird on stack mode copy
  drm/msm: Use drm_mode_init() for on-stack modes
  drm/msm: Use drm_mode_copy()
  drm/mtk: Use drm_mode_init() for on-stack modes
  drm/rockchip: Use drm_mode_copy()
  drm/sti: Use drm_mode_copy()
  drm/tilcdc: Use drm_mode_copy()
  drm/vc4: Use drm_mode_copy()
  drm/i915: Use drm_mode_init() for on-stack modes
  drm/i915: Use drm_mode_copy()
  drm/panel: Use drm_mode_duplicate()
  drm: Use drm_mode_init() for on-stack modes
  drm: Use drm_mode_copy()

 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    |  4 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 ++++++++++---------
 drivers/gpu/drm/bridge/nwl-dsi.c              |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     |  2 +-
 drivers/gpu/drm/bridge/tc358767.c             |  2 +-
 drivers/gpu/drm/drm_crtc_helper.c             | 12 +++---
 drivers/gpu/drm/drm_edid.c                    |  8 +++-
 drivers/gpu/drm/drm_modes.c                   | 21 +++++++++-
 drivers/gpu/drm/drm_vblank.c                  |  2 +-
 drivers/gpu/drm/gma500/oaktrail_crtc.c        |  8 +---
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c  |  2 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 20 +++++----
 drivers/gpu/drm/imx/imx-ldb.c                 |  3 +-
 drivers/gpu/drm/mediatek/mtk_hdmi.c           |  2 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  |  2 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |  9 ++--
 drivers/gpu/drm/msm/dp/dp_display.c           |  2 +-
 drivers/gpu/drm/msm/dp/dp_drm.c               | 10 ++---
 drivers/gpu/drm/panel/panel-truly-nt35597.c   |  3 +-
 .../gpu/drm/panel/panel-visionox-rm69299.c    |  4 +-
 drivers/gpu/drm/radeon/radeon_connectors.c    |  4 +-
 drivers/gpu/drm/rockchip/cdn-dp-core.c        |  2 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c          |  2 +-
 drivers/gpu/drm/rockchip/rk3066_hdmi.c        |  2 +-
 drivers/gpu/drm/sti/sti_dvo.c                 |  2 +-
 drivers/gpu/drm/sti/sti_hda.c                 |  2 +-
 drivers/gpu/drm/sti/sti_hdmi.c                |  2 +-
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c          |  2 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c                |  5 +--
 include/drm/drm_modes.h                       |  2 +
 30 files changed, 105 insertions(+), 79 deletions(-)

Comments

Ville Syrjälä March 14, 2022, 10:11 p.m. UTC | #1
On Fri, Feb 18, 2022 at 12:03:41PM +0200, Ville Syrjala wrote:
>   drm: Add drm_mode_init()
>   drm/bridge: Use drm_mode_copy()
>   drm/imx: Use drm_mode_duplicate()
>   drm/panel: Use drm_mode_duplicate()
>   drm/vc4: Use drm_mode_copy()
These have been pushed to drm-misc-next.

>   drm/amdgpu: Remove pointless on stack mode copies
>   drm/amdgpu: Use drm_mode_init() for on-stack modes
>   drm/amdgpu: Use drm_mode_copy()
amdgpu ones are reviewed, but I'll leave them for the
AMD folks to push to whichever tree they prefer.


The rest are still in need of review:
>   drm/radeon: Use drm_mode_copy()
>   drm/gma500: Use drm_mode_copy()
>   drm/hisilicon: Use drm_mode_init() for on-stack modes
>   drm/msm: Nuke weird on stack mode copy
>   drm/msm: Use drm_mode_init() for on-stack modes
>   drm/msm: Use drm_mode_copy()
>   drm/mtk: Use drm_mode_init() for on-stack modes
>   drm/rockchip: Use drm_mode_copy()
>   drm/sti: Use drm_mode_copy()
>   drm/tilcdc: Use drm_mode_copy()
>   drm/i915: Use drm_mode_init() for on-stack modes
>   drm/i915: Use drm_mode_copy()
>   drm: Use drm_mode_init() for on-stack modes
>   drm: Use drm_mode_copy()
Alex Deucher March 15, 2022, 6:52 p.m. UTC | #2
On Mon, Mar 14, 2022 at 6:12 PM Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Fri, Feb 18, 2022 at 12:03:41PM +0200, Ville Syrjala wrote:
> >   drm: Add drm_mode_init()
> >   drm/bridge: Use drm_mode_copy()
> >   drm/imx: Use drm_mode_duplicate()
> >   drm/panel: Use drm_mode_duplicate()
> >   drm/vc4: Use drm_mode_copy()
> These have been pushed to drm-misc-next.
>
> >   drm/amdgpu: Remove pointless on stack mode copies
> >   drm/amdgpu: Use drm_mode_init() for on-stack modes
> >   drm/amdgpu: Use drm_mode_copy()
> amdgpu ones are reviewed, but I'll leave them for the
> AMD folks to push to whichever tree they prefer.

I pulled patches 2, 4, 5 into my tree.  For 3, I'm happy to have it
land via drm-misc with the rest of the mode_init changes if you'd
prefer.

Alex


Alex

>
>
> The rest are still in need of review:
> >   drm/radeon: Use drm_mode_copy()
> >   drm/gma500: Use drm_mode_copy()
> >   drm/hisilicon: Use drm_mode_init() for on-stack modes
> >   drm/msm: Nuke weird on stack mode copy
> >   drm/msm: Use drm_mode_init() for on-stack modes
> >   drm/msm: Use drm_mode_copy()
> >   drm/mtk: Use drm_mode_init() for on-stack modes
> >   drm/rockchip: Use drm_mode_copy()
> >   drm/sti: Use drm_mode_copy()
> >   drm/tilcdc: Use drm_mode_copy()
> >   drm/i915: Use drm_mode_init() for on-stack modes
> >   drm/i915: Use drm_mode_copy()
> >   drm: Use drm_mode_init() for on-stack modes
> >   drm: Use drm_mode_copy()
>
> --
> Ville Syrjälä
> Intel
Ville Syrjälä March 21, 2022, 10:37 p.m. UTC | #3
On Tue, Mar 15, 2022 at 02:52:38PM -0400, Alex Deucher wrote:
> On Mon, Mar 14, 2022 at 6:12 PM Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Fri, Feb 18, 2022 at 12:03:41PM +0200, Ville Syrjala wrote:
> > >   drm: Add drm_mode_init()
> > >   drm/bridge: Use drm_mode_copy()
> > >   drm/imx: Use drm_mode_duplicate()
> > >   drm/panel: Use drm_mode_duplicate()
> > >   drm/vc4: Use drm_mode_copy()
> > These have been pushed to drm-misc-next.
> >
> > >   drm/amdgpu: Remove pointless on stack mode copies
> > >   drm/amdgpu: Use drm_mode_init() for on-stack modes
> > >   drm/amdgpu: Use drm_mode_copy()
> > amdgpu ones are reviewed, but I'll leave them for the
> > AMD folks to push to whichever tree they prefer.
> 
> I pulled patches 2, 4, 5 into my tree.

Thanks.

> For 3, I'm happy to have it
> land via drm-misc with the rest of the mode_init changes if you'd
> prefer.

Either way works for me. I don't yet have reviews yet for
the other drivers, so I'll proably hold off for a bit more
at least. And the i915 patch I'll be merging via drm-intel.

> > >   drm/radeon: Use drm_mode_copy()
> > >   drm/gma500: Use drm_mode_copy()
> > >   drm/tilcdc: Use drm_mode_copy()
> > >   drm/i915: Use drm_mode_copy()

Those are now all in.

Which leaves us with these stragglers:
> > >   drm/hisilicon: Use drm_mode_init() for on-stack modes
> > >   drm/msm: Nuke weird on stack mode copy
> > >   drm/msm: Use drm_mode_init() for on-stack modes
> > >   drm/msm: Use drm_mode_copy()
> > >   drm/mtk: Use drm_mode_init() for on-stack modes
> > >   drm/rockchip: Use drm_mode_copy()
> > >   drm/sti: Use drm_mode_copy()
> > >   drm: Use drm_mode_init() for on-stack modes
> > >   drm: Use drm_mode_copy()
Dmitry Baryshkov March 23, 2022, 10:39 a.m. UTC | #4
On 22/03/2022 01:37, Ville Syrjälä wrote:
> On Tue, Mar 15, 2022 at 02:52:38PM -0400, Alex Deucher wrote:
>> On Mon, Mar 14, 2022 at 6:12 PM Ville Syrjälä
>> <ville.syrjala@linux.intel.com> wrote:
>>>
>>> On Fri, Feb 18, 2022 at 12:03:41PM +0200, Ville Syrjala wrote:
>>>>    drm: Add drm_mode_init()
>>>>    drm/bridge: Use drm_mode_copy()
>>>>    drm/imx: Use drm_mode_duplicate()
>>>>    drm/panel: Use drm_mode_duplicate()
>>>>    drm/vc4: Use drm_mode_copy()
>>> These have been pushed to drm-misc-next.
>>>
>>>>    drm/amdgpu: Remove pointless on stack mode copies
>>>>    drm/amdgpu: Use drm_mode_init() for on-stack modes
>>>>    drm/amdgpu: Use drm_mode_copy()
>>> amdgpu ones are reviewed, but I'll leave them for the
>>> AMD folks to push to whichever tree they prefer.
>>
>> I pulled patches 2, 4, 5 into my tree.
> 
> Thanks.
> 
>> For 3, I'm happy to have it
>> land via drm-misc with the rest of the mode_init changes if you'd
>> prefer.
> 
> Either way works for me. I don't yet have reviews yet for
> the other drivers, so I'll proably hold off for a bit more
> at least. And the i915 patch I'll be merging via drm-intel.
> 
>>>>    drm/radeon: Use drm_mode_copy()
>>>>    drm/gma500: Use drm_mode_copy()
>>>>    drm/tilcdc: Use drm_mode_copy()
>>>>    drm/i915: Use drm_mode_copy()
> 
> Those are now all in.
> 
> Which leaves us with these stragglers:
>>>>    drm/hisilicon: Use drm_mode_init() for on-stack modes

>>>>    drm/msm: Nuke weird on stack mode copy
>>>>    drm/msm: Use drm_mode_init() for on-stack modes
>>>>    drm/msm: Use drm_mode_copy()

These three patches went beneath my radar for some reason.

I have just sent a replacement for the first patch ([1]). Other two 
patches can be pulled in msm/msm-next (or msm/msm-fixes) during the next 
development cycle if that works for you.

[1] https://patchwork.freedesktop.org/series/101682/

>>>>    drm/mtk: Use drm_mode_init() for on-stack modes
>>>>    drm/rockchip: Use drm_mode_copy()
>>>>    drm/sti: Use drm_mode_copy()
>>>>    drm: Use drm_mode_init() for on-stack modes
>>>>    drm: Use drm_mode_copy()
>
Ville Syrjälä March 23, 2022, 3:10 p.m. UTC | #5
On Wed, Mar 23, 2022 at 01:39:44PM +0300, Dmitry Baryshkov wrote:
> On 22/03/2022 01:37, Ville Syrjälä wrote:
> > On Tue, Mar 15, 2022 at 02:52:38PM -0400, Alex Deucher wrote:
> >> On Mon, Mar 14, 2022 at 6:12 PM Ville Syrjälä
> >> <ville.syrjala@linux.intel.com> wrote:
> >>>
> >>> On Fri, Feb 18, 2022 at 12:03:41PM +0200, Ville Syrjala wrote:
> >>>>    drm: Add drm_mode_init()
> >>>>    drm/bridge: Use drm_mode_copy()
> >>>>    drm/imx: Use drm_mode_duplicate()
> >>>>    drm/panel: Use drm_mode_duplicate()
> >>>>    drm/vc4: Use drm_mode_copy()
> >>> These have been pushed to drm-misc-next.
> >>>
> >>>>    drm/amdgpu: Remove pointless on stack mode copies
> >>>>    drm/amdgpu: Use drm_mode_init() for on-stack modes
> >>>>    drm/amdgpu: Use drm_mode_copy()
> >>> amdgpu ones are reviewed, but I'll leave them for the
> >>> AMD folks to push to whichever tree they prefer.
> >>
> >> I pulled patches 2, 4, 5 into my tree.
> > 
> > Thanks.
> > 
> >> For 3, I'm happy to have it
> >> land via drm-misc with the rest of the mode_init changes if you'd
> >> prefer.
> > 
> > Either way works for me. I don't yet have reviews yet for
> > the other drivers, so I'll proably hold off for a bit more
> > at least. And the i915 patch I'll be merging via drm-intel.
> > 
> >>>>    drm/radeon: Use drm_mode_copy()
> >>>>    drm/gma500: Use drm_mode_copy()
> >>>>    drm/tilcdc: Use drm_mode_copy()
> >>>>    drm/i915: Use drm_mode_copy()
> > 
> > Those are now all in.
> > 
> > Which leaves us with these stragglers:
> >>>>    drm/hisilicon: Use drm_mode_init() for on-stack modes
> 
> >>>>    drm/msm: Nuke weird on stack mode copy
> >>>>    drm/msm: Use drm_mode_init() for on-stack modes
> >>>>    drm/msm: Use drm_mode_copy()
> 
> These three patches went beneath my radar for some reason.
> 
> I have just sent a replacement for the first patch ([1]). Other two 
> patches can be pulled in msm/msm-next (or msm/msm-fixes) during the next 
> development cycle if that works for you.

That'll do fine for me. Thanks.

> 
> [1] https://patchwork.freedesktop.org/series/101682/
> 
> >>>>    drm/mtk: Use drm_mode_init() for on-stack modes
> >>>>    drm/rockchip: Use drm_mode_copy()
> >>>>    drm/sti: Use drm_mode_copy()
> >>>>    drm: Use drm_mode_init() for on-stack modes
> >>>>    drm: Use drm_mode_copy()
> > 
> 
> 
> -- 
> With best wishes
> Dmitry
Abhinav Kumar March 23, 2022, 8:50 p.m. UTC | #6
On 3/23/2022 3:39 AM, Dmitry Baryshkov wrote:
> On 22/03/2022 01:37, Ville Syrjälä wrote:
>> On Tue, Mar 15, 2022 at 02:52:38PM -0400, Alex Deucher wrote:
>>> On Mon, Mar 14, 2022 at 6:12 PM Ville Syrjälä
>>> <ville.syrjala@linux.intel.com> wrote:
>>>>
>>>> On Fri, Feb 18, 2022 at 12:03:41PM +0200, Ville Syrjala wrote:
>>>>>    drm: Add drm_mode_init()
>>>>>    drm/bridge: Use drm_mode_copy()
>>>>>    drm/imx: Use drm_mode_duplicate()
>>>>>    drm/panel: Use drm_mode_duplicate()
>>>>>    drm/vc4: Use drm_mode_copy()
>>>> These have been pushed to drm-misc-next.
>>>>
>>>>>    drm/amdgpu: Remove pointless on stack mode copies
>>>>>    drm/amdgpu: Use drm_mode_init() for on-stack modes
>>>>>    drm/amdgpu: Use drm_mode_copy()
>>>> amdgpu ones are reviewed, but I'll leave them for the
>>>> AMD folks to push to whichever tree they prefer.
>>>
>>> I pulled patches 2, 4, 5 into my tree.
>>
>> Thanks.
>>
>>> For 3, I'm happy to have it
>>> land via drm-misc with the rest of the mode_init changes if you'd
>>> prefer.
>>
>> Either way works for me. I don't yet have reviews yet for
>> the other drivers, so I'll proably hold off for a bit more
>> at least. And the i915 patch I'll be merging via drm-intel.
>>
>>>>>    drm/radeon: Use drm_mode_copy()
>>>>>    drm/gma500: Use drm_mode_copy()
>>>>>    drm/tilcdc: Use drm_mode_copy()
>>>>>    drm/i915: Use drm_mode_copy()
>>
>> Those are now all in.
>>
>> Which leaves us with these stragglers:
>>>>>    drm/hisilicon: Use drm_mode_init() for on-stack modes
> 
>>>>>    drm/msm: Nuke weird on stack mode copy
>>>>>    drm/msm: Use drm_mode_init() for on-stack modes
>>>>>    drm/msm: Use drm_mode_copy()
> 
> These three patches went beneath my radar for some reason.
> 
> I have just sent a replacement for the first patch ([1]). Other two 
> patches can be pulled in msm/msm-next (or msm/msm-fixes) during the next 
> development cycle if that works for you.
> 
> [1] https://patchwork.freedesktop.org/series/101682/

Agree with this approach.

We can replace the first patch with 
https://patchwork.freedesktop.org/series/101682/.

Thanks

Abhinav

> 
>>>>>    drm/mtk: Use drm_mode_init() for on-stack modes
>>>>>    drm/rockchip: Use drm_mode_copy()
>>>>>    drm/sti: Use drm_mode_copy()
>>>>>    drm: Use drm_mode_init() for on-stack modes
>>>>>    drm: Use drm_mode_copy()
>>
> 
>