[PATCH 05/14] drm/vc4: drv: Register a different driver on BCM2711

[Melissa Wen]

On 05/03, Maxime Ripard wrote:
> Prior to the BCM2711/RaspberryPi4, the GPU was a part of the display
> components of the SoC. It was thus a part of the vc4 driver.
> 
> However, with the BCM2711, it got split out and thus the v3d driver was
> created. The vc4 driver now only handles the display part.
> 
> We didn't properly split out the code when doing the BCM2711 support
> though, and most of the code around buffer allocations is still
> involved, even though it doesn't have the backing hardware anymore.
> 
> Let's start the split out by creating a new drm_driver that only reports
> and uses what we support on the BCM2711. The ioctl were properly
> filtered already, but we were still exposing a .gem_create_object hook,
> as well as having an .open and .postclose hooks which are only relevant
> on older generations.
> 
> Signed-off-by: Maxime Ripard <maxime at cerno.tech>
> ---
>  drivers/gpu/drm/vc4/vc4_drv.c | 51 ++++++++++++++++++++++++++++-------
>  1 file changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index eb08940028d3..4f9e2067dad0 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -76,6 +76,19 @@ int vc4_dumb_fixup_args(struct drm_mode_create_dumb *args)
>  	return 0;
>  }
>  
> +static int vc4_dumb_create(struct drm_file *file_priv,
> +			   struct drm_device *dev,
> +			   struct drm_mode_create_dumb *args)
> +{
> +	int ret;
> +
> +	ret = vc4_dumb_fixup_args(args);
> +	if (ret)
> +		return ret;
> +
> +	return drm_gem_cma_dumb_create_internal(file_priv, dev, args);
> +}
> +
>  static int vc4_get_param_ioctl(struct drm_device *dev, void *data,
>  			       struct drm_file *file_priv)
>  {
> @@ -173,7 +186,7 @@ static const struct drm_ioctl_desc vc4_drm_ioctls[] = {
>  	DRM_IOCTL_DEF_DRV(VC4_PERFMON_GET_VALUES, vc4_perfmon_get_values_ioctl, DRM_RENDER_ALLOW),
>  };
>  
> -static struct drm_driver vc4_drm_driver = {
> +static const struct drm_driver vc4_drm_driver = {
>  	.driver_features = (DRIVER_MODESET |
>  			    DRIVER_ATOMIC |
>  			    DRIVER_GEM |
> @@ -202,6 +215,27 @@ static struct drm_driver vc4_drm_driver = {
>  	.patchlevel = DRIVER_PATCHLEVEL,
>  };
>  
> +static const struct drm_driver vc5_drm_driver = {
> +	.driver_features = (DRIVER_MODESET |
> +			    DRIVER_ATOMIC |
> +			    DRIVER_GEM),
> +
> +#if defined(CONFIG_DEBUG_FS)
> +	.debugfs_init = vc4_debugfs_init,
> +#endif
> +
> +	DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(vc4_dumb_create),

I wonder if we can call it `vc5_dumb_create` to dissociate from vc4.
Instead of a `vc4_dumb_create` only used by vc5_drm_driver.

I mean, mistunderstandings already happen between vc4->v3d (component)
and v3d (driver). Then now we have a vc5 - without v3d (component) and
existing a v3d (driver) - and now I worry that it's going to get even
more confusing...

> +
> +	.fops = &vc4_drm_fops,
> +
> +	.name = DRIVER_NAME,
> +	.desc = DRIVER_DESC,
> +	.date = DRIVER_DATE,
> +	.major = DRIVER_MAJOR,
> +	.minor = DRIVER_MINOR,
> +	.patchlevel = DRIVER_PATCHLEVEL,
> +};
> +
>  static void vc4_match_add_drivers(struct device *dev,
>  				  struct component_match **match,
>  				  struct platform_driver *const *drivers,
> @@ -225,6 +259,7 @@ static void vc4_match_add_drivers(struct device *dev,
>  static int vc4_drm_bind(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> +	const struct drm_driver *driver;
>  	struct rpi_firmware *firmware = NULL;
>  	struct drm_device *drm;
>  	struct vc4_dev *vc4;
> @@ -236,14 +271,12 @@ static int vc4_drm_bind(struct device *dev)
>  	dev->coherent_dma_mask = DMA_BIT_MASK(32);
>  
>  	is_vc5 = of_device_is_compatible(dev->of_node, "brcm,bcm2711-vc5");
> +	if (is_vc5)
> +		driver = &vc5_drm_driver;
> +	else
> +		driver = &vc4_drm_driver;
>  
> -	/* If VC4 V3D is missing, don't advertise render nodes. */
> -	node = of_find_matching_node_and_match(NULL, vc4_v3d_dt_match, NULL);
> -	if (!node || !of_device_is_available(node))
> -		vc4_drm_driver.driver_features &= ~DRIVER_RENDER;
> -	of_node_put(node);
> -
> -	vc4 = devm_drm_dev_alloc(dev, &vc4_drm_driver, struct vc4_dev, base);
> +	vc4 = devm_drm_dev_alloc(dev, driver, struct vc4_dev, base);
>  	if (IS_ERR(vc4))
>  		return PTR_ERR(vc4);
>  	vc4->is_vc5 = is_vc5;
> @@ -275,7 +308,7 @@ static int vc4_drm_bind(struct device *dev)
>  			return -EPROBE_DEFER;
>  	}
>  
> -	ret = drm_aperture_remove_framebuffers(false, &vc4_drm_driver);
> +	ret = drm_aperture_remove_framebuffers(false, driver);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.35.1
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220509/bb307684/attachment-0001.sig>