24.01.2019 21:02, Thierry Reding пишет:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> 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@xxxxxxxxx>.
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> 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@xxxxxxxxx>
