Re: [PATCH 00/14] drm/vc4: Properly separate v3d on BCM2711, and fix frame ordering

2022-05-09 Thread Melissa Wen
On 05/03, Maxime Ripard wrote:
> Hi,
> 
> Here's a series that fixes a significant issue we missed when adding support
> for the BCM2711 / RaspberryPi4 in the vc4 driver.
> 
> Indeed, before the introduction of the BCM2711 support, the GPU was fairly
> intertwined with the display hardware, and was thus supported by the vc4
> driver. Among other things, the driver needed to accomodate for a bunch of
> hardware limitations (such as a lack of IOMMU) by implementing a custom memory
> management backend, tied with the v3d hardware.
> 
> On the BCM2711, that GPU got moved into a completely separate hardware block
> and thus we gained a new driver for it, v3d.
> 
> However, when we introduced the display support for the BCM2711 in vc4, we
> didn't properly split out the v3d-related functions and ended up reusing a
> significant portion of the code supposed to be backed by the v3d.
> 
> This created a bunch of easy to miss issues that would only pop up with IGT
> tests, or when heavily testing some features (like asynchronous 
> page-flipping).
> 
> This series properly does the split now by creating separate code path where
> relevant, adds a loud complain when we use a v3d entry-point on the BCM2711,
> and fixes an issue where we would just ignore any fence on an asynchronous
> page-flip, resulting in frames appearing out-of-order for some workloads.
> 
> Let me know what you think,

Hi Maxime,

Overall lgtm. I didn't catch anything tricky and I did some tests to
verify the current behaviour is preserved. Unfortunately, I couldn't
test the async mechanisms much, let me know if you have any suggestions
for testing. I only concern about accommodate vc5 naming since vc4->v3d
vs v3d is already a little confusing. After addressing
dma_resv_get_singleton usage, this series is:

Reviewed-by: Melissa Wen 

> Maxime
> 
> Maxime Ripard (14):
>   drm/vc4: plane: Prevent async update if we don't have a dlist
>   drm/vc4: Consolidate Hardware Revision Check
>   drm/vc4: bo: Rename vc4_dumb_create
>   drm/vc4: bo: Split out Dumb buffers fixup
>   drm/vc4: drv: Register a different driver on BCM2711
>   drm/vc4: kms: Register a different drm_mode_config_funcs on BCM2711
>   drm/vc4: plane: Register a different drm_plane_helper_funcs on BCM2711
>   drm/vc4: drv: Skip BO Backend Initialization on BCM2711
>   drm/vc4: crtc: Use an union to store the page flip callback
>   drm/vc4: crtc: Move the BO handling out of common page-flip callback
>   drm/vc4: crtc: Move the BO Handling out of Common Page-Flip Handler
>   drm/vc4: crtc: Don't call into BO Handling on Async Page-Flips on
> BCM2711
>   drm/vc4: crtc: Fix out of order frames during asynchronous page flips
>   drm/vc4: Warn if some v3d code is run on BCM2711
> 
>  drivers/gpu/drm/vc4/vc4_bo.c   |  62 ++-
>  drivers/gpu/drm/vc4/vc4_crtc.c | 193 +++--
>  drivers/gpu/drm/vc4/vc4_drv.c  |  97 +--
>  drivers/gpu/drm/vc4/vc4_drv.h  |  19 +-
>  drivers/gpu/drm/vc4/vc4_gem.c  |  40 +
>  drivers/gpu/drm/vc4/vc4_hvs.c  |  18 +-
>  drivers/gpu/drm/vc4/vc4_irq.c  |  16 ++
>  drivers/gpu/drm/vc4/vc4_kms.c  |  24 ++-
>  drivers/gpu/drm/vc4/vc4_perfmon.c  |  41 +
>  drivers/gpu/drm/vc4/vc4_plane.c|  29 +++-
>  drivers/gpu/drm/vc4/vc4_render_cl.c|   4 +
>  drivers/gpu/drm/vc4/vc4_v3d.c  |  15 ++
>  drivers/gpu/drm/vc4/vc4_validate.c |  16 ++
>  drivers/gpu/drm/vc4/vc4_validate_shaders.c |   4 +
>  14 files changed, 470 insertions(+), 108 deletions(-)
> 
> -- 
> 2.35.1
> 


signature.asc
Description: PGP signature


[PATCH 00/14] drm/vc4: Properly separate v3d on BCM2711, and fix frame ordering

2022-05-03 Thread Maxime Ripard
Hi,

Here's a series that fixes a significant issue we missed when adding support
for the BCM2711 / RaspberryPi4 in the vc4 driver.

Indeed, before the introduction of the BCM2711 support, the GPU was fairly
intertwined with the display hardware, and was thus supported by the vc4
driver. Among other things, the driver needed to accomodate for a bunch of
hardware limitations (such as a lack of IOMMU) by implementing a custom memory
management backend, tied with the v3d hardware.

On the BCM2711, that GPU got moved into a completely separate hardware block
and thus we gained a new driver for it, v3d.

However, when we introduced the display support for the BCM2711 in vc4, we
didn't properly split out the v3d-related functions and ended up reusing a
significant portion of the code supposed to be backed by the v3d.

This created a bunch of easy to miss issues that would only pop up with IGT
tests, or when heavily testing some features (like asynchronous page-flipping).

This series properly does the split now by creating separate code path where
relevant, adds a loud complain when we use a v3d entry-point on the BCM2711,
and fixes an issue where we would just ignore any fence on an asynchronous
page-flip, resulting in frames appearing out-of-order for some workloads.

Let me know what you think,
Maxime

Maxime Ripard (14):
  drm/vc4: plane: Prevent async update if we don't have a dlist
  drm/vc4: Consolidate Hardware Revision Check
  drm/vc4: bo: Rename vc4_dumb_create
  drm/vc4: bo: Split out Dumb buffers fixup
  drm/vc4: drv: Register a different driver on BCM2711
  drm/vc4: kms: Register a different drm_mode_config_funcs on BCM2711
  drm/vc4: plane: Register a different drm_plane_helper_funcs on BCM2711
  drm/vc4: drv: Skip BO Backend Initialization on BCM2711
  drm/vc4: crtc: Use an union to store the page flip callback
  drm/vc4: crtc: Move the BO handling out of common page-flip callback
  drm/vc4: crtc: Move the BO Handling out of Common Page-Flip Handler
  drm/vc4: crtc: Don't call into BO Handling on Async Page-Flips on
BCM2711
  drm/vc4: crtc: Fix out of order frames during asynchronous page flips
  drm/vc4: Warn if some v3d code is run on BCM2711

 drivers/gpu/drm/vc4/vc4_bo.c   |  62 ++-
 drivers/gpu/drm/vc4/vc4_crtc.c | 193 +++--
 drivers/gpu/drm/vc4/vc4_drv.c  |  97 +--
 drivers/gpu/drm/vc4/vc4_drv.h  |  19 +-
 drivers/gpu/drm/vc4/vc4_gem.c  |  40 +
 drivers/gpu/drm/vc4/vc4_hvs.c  |  18 +-
 drivers/gpu/drm/vc4/vc4_irq.c  |  16 ++
 drivers/gpu/drm/vc4/vc4_kms.c  |  24 ++-
 drivers/gpu/drm/vc4/vc4_perfmon.c  |  41 +
 drivers/gpu/drm/vc4/vc4_plane.c|  29 +++-
 drivers/gpu/drm/vc4/vc4_render_cl.c|   4 +
 drivers/gpu/drm/vc4/vc4_v3d.c  |  15 ++
 drivers/gpu/drm/vc4/vc4_validate.c |  16 ++
 drivers/gpu/drm/vc4/vc4_validate_shaders.c |   4 +
 14 files changed, 470 insertions(+), 108 deletions(-)

-- 
2.35.1