Re: [PATCH v2 08/13] drm/tegra: vic: Load firmware on demand

[Dmitry Osipenko <digetx@gmail.com>]

24.01.2019 21:02, Thierry Reding пишет:
> From: Thierry Reding <treding@nvidia.com>
> 
> Loading the firmware requires an allocation of IOVA space to make sure
> that the VIC's Falcon microcontroller can read the firmware if address
> translation via the SMMU is enabled.
> 
> However, the allocation currently happens at a time where the geometry
> of an IOMMU domain may not have been initialized yet. This happens for
> example on Tegra186 and later where an ARM SMMU is used. Domains which
> are created by the ARM SMMU driver postpone the geometry setup until a
> device is attached to the domain. This is because IOMMU domains aren't
> attached to a specific IOMMU instance at allocation time and hence the
> input address space, which defines the geometry, is not known yet.
> 
> Work around this by postponing the firmware load until it is needed at
> the time where a channel is opened to the VIC. At this time the shared
> IOMMU domain's geometry has been properly initialized.
> 
> As a byproduct this allows the Tegra DRM to be created in the absence
> of VIC firmware, since the VIC initialization no longer fails if the
> firmware can't be found.
> 
> Based on an earlier patch by Dmitry Osipenko <digetx@gmail.com>.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - actually request firmware on demand
> 
>  drivers/gpu/drm/tegra/vic.c | 53 +++++++++++++++++++++++++------------
>  1 file changed, 36 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> index d47983deb1cf..9cb10d1e923a 100644
> --- a/drivers/gpu/drm/tegra/vic.c
> +++ b/drivers/gpu/drm/tegra/vic.c
> @@ -181,13 +181,6 @@ static int vic_init(struct host1x_client *client)
>  		vic->domain = tegra->domain;
>  	}
>  
> -	if (!vic->falcon.data) {
> -		vic->falcon.data = tegra;
> -		err = falcon_load_firmware(&vic->falcon);
> -		if (err < 0)
> -			goto detach;
> -	}
> -
>  	vic->channel = host1x_channel_request(client->dev);
>  	if (!vic->channel) {
>  		err = -ENOMEM;
> @@ -246,6 +239,30 @@ static const struct host1x_client_ops vic_client_ops = {
>  	.exit = vic_exit,
>  };
>  
> +static int vic_load_firmware(struct vic *vic)
> +{
> +	int err;
> +
> +	if (vic->falcon.data)
> +		return 0;
> +
> +	vic->falcon.data = vic->client->drm;
> +
> +	err = falcon_read_firmware(&vic->falcon, vic->config->firmware);
> +	if (err < 0)
> +		goto cleanup;
> +
> +	err = falcon_load_firmware(&vic->falcon);
> +	if (err < 0)
> +		goto cleanup;
> +
> +	return 0;
> +
> +cleanup:
> +	vic->falcon.data = NULL;
> +	return err;
> +}
> +
>  static int vic_open_channel(struct tegra_drm_client *client,
>  			    struct tegra_drm_context *context)
>  {
> @@ -256,19 +273,25 @@ static int vic_open_channel(struct tegra_drm_client *client,
>  	if (err < 0)
>  		return err;
>  
> +	err = vic_load_firmware(vic);
> +	if (err < 0)
> +		goto rpm_put;
> +
>  	err = vic_boot(vic);
> -	if (err < 0) {
> -		pm_runtime_put(vic->dev);
> -		return err;
> -	}
> +	if (err < 0)
> +		goto rpm_put;
>  
>  	context->channel = host1x_channel_get(vic->channel);
>  	if (!context->channel) {
> -		pm_runtime_put(vic->dev);
> -		return -ENOMEM;
> +		err = -ENOMEM;
> +		goto rpm_put;
>  	}
>  
>  	return 0;
> +
> +rpm_put:
> +	pm_runtime_put(vic->dev);
> +	return err;
>  }
>  
>  static void vic_close_channel(struct tegra_drm_context *context)
> @@ -372,10 +395,6 @@ static int vic_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		return err;
>  
> -	err = falcon_read_firmware(&vic->falcon, vic->config->firmware);
> -	if (err < 0)
> -		goto exit_falcon;
> -
>  	platform_set_drvdata(pdev, vic);
>  
>  	INIT_LIST_HEAD(&vic->client.base.list);
> 

Looks good this time!

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel