Re: [PATCH v2 5/9] drm/msm: add headless gpu device (for imx5)

On Wed, Nov 21, 2018 at 8:55 PM Jonathan Marek jonathan@marek.ca wrote:

This patch allows using drm/msm without qcom display hardware. This is especially useful for iMX5 hardware, which has a a2xx GPU but uses the imx-drm driver for display.

Signed-off-by: Jonathan Marek jonathan@marek.ca

v2: added commit message and removed unnecessary comment

drivers/gpu/drm/msm/Kconfig | 4 ++-- drivers/gpu/drm/msm/msm_debugfs.c | 2 +- drivers/gpu/drm/msm/msm_drv.c | 21 +++++++++++---------- 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 843a9d40c05e..cf549f1ed403 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -2,7 +2,7 @@ config DRM_MSM tristate "MSM DRM" depends on DRM

•   depends on ARCH_QCOM || (ARM && COMPILE_TEST)
  

•   depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)
    depends on OF && COMMON_CLK
    depends on MMU
    select QCOM_MDT_LOADER if ARCH_QCOM
  

@@ -11,7 +11,7 @@ config DRM_MSM select DRM_PANEL select SHMEM select TMPFS

•   select QCOM_SCM
  

•   select QCOM_SCM if ARCH_QCOM
    select WANT_DEV_COREDUMP
    select SND_SOC_HDMI_CODEC if SND_SOC
    select SYNC_FILE
  

diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c index f0da0d3c8a80..1ca99ca356a4 100644 --- a/drivers/gpu/drm/msm/msm_debugfs.c +++ b/drivers/gpu/drm/msm/msm_debugfs.c @@ -235,7 +235,7 @@ int msm_debugfs_init(struct drm_minor *minor) debugfs_create_file("gpu", S_IRUSR, minor->debugfs_root, dev, &msm_gpu_fops);

•   if (priv->kms->funcs->debugfs_init) {
  

•   if (priv->kms && priv->kms->funcs->debugfs_init) {
            ret = priv->kms->funcs->debugfs_init(priv->kms, minor);
            if (ret)
                    return ret;
  

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 9f219e02f3c7..3f35e57202ef 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -344,6 +344,7 @@ static int msm_drm_uninit(struct device *dev) return 0; }

+#define KMS_HEADLESS 1 #define KMS_MDP4 4 #define KMS_MDP5 5 #define KMS_DPU 3 @@ -495,6 +496,9 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) msm_gem_shrinker_init(ddev);

    switch (get_mdp_ver(pdev)) {

•   case KMS_HEADLESS:
  

•           priv->kms = kms = NULL;
  

•           break;
    case KMS_MDP4:
            kms = mdp4_kms_init(ddev);
            priv->kms = kms;
  

@@ -512,12 +516,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) }

    if (IS_ERR(kms)) {

•           /*
  

•            * NOTE: once we have GPU support, having no kms should not
  

•            * be considered fatal.. ideally we would still support gpu
  

•            * and (for example) use dmabuf/prime to share buffers with
  

•            * imx drm driver on iMX5
  

•            */
            dev_err(dev, "failed to load kms\n");
            ret = PTR_ERR(kms);
            goto err_msm_uninit;
  

@@ -633,7 +631,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) drm_mode_config_reset(ddev);

#ifdef CONFIG_DRM_FBDEV_EMULATION

•   if (fbdev)
  

•   if (kms && fbdev)
            priv->fbdev = msm_fbdev_init(ddev);
  

#endif

@@ -1315,9 +1313,11 @@ static int msm_pdev_probe(struct platform_device *pdev) struct component_match *match = NULL; int ret;

•   ret = add_display_components(&pdev->dev, &match);
  

•   if (ret)
  

•           return ret;
  

•   if (get_mdp_ver(pdev) != KMS_HEADLESS) {
  

•           ret = add_display_components(&pdev->dev, &match);
  

•           if (ret)
  

•                   return ret;
  

•   }
  
    ret = add_gpu_components(&pdev->dev, &match);
    if (ret)
  

@@ -1342,6 +1342,7 @@ static int msm_pdev_remove(struct platform_device *pdev) }

static const struct of_device_id dt_match[] = {

•   { .compatible = "qcom,adreno-headless", .data = (void *)KMS_HEADLESS },
  

so, I don't really think we should have a "headless" dt node, because that doesn't really represent the hw. Maybe instead for the gpu device node, we could use a different compatible string like:

gpu@xyz { compatible = "amd,imageon-205.0", "amd,imageon"; ... };

(ie. amd,imageon instead of qcom,adreno)

Then in adreno_bind() (or somewhere around there), if we have the amd,imageon compat string instead of qcom,adreno, then programatically create a dummy "headless" device that the toplevel drm driver can bind to. I think something like this should be possible, and avoid needing a qcom,adreno-headless node in dt.

BR, -R

    { .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
    { .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
    { .compatible = "qcom,sdm845-mdss", .data = (void *)KMS_DPU },

-- 2.17.1