[PATCH 2/2] drm/msm: Decouple hdmi driver from mdp driver

This change is to remove the hdmi structure from mdp kms data structure.

To do this, the initialization flow is re-arranged. - hdmi_init is moved from modeset_init to hdmi_bind. - hdmi_destroy is called by hdmi_unbind and the use of kref is abandoned. - A new interface hdmi_set_encoder is introduced to establish the links between hdmi connector, encoder and bridge.

Signed-off-by: Hai Li hali@codeaurora.org --- drivers/gpu/drm/msm/hdmi/hdmi.c | 61 +++++++++++++++++++++++-------- drivers/gpu/drm/msm/hdmi/hdmi.h | 14 ------- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 3 +- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 6 +-- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 14 ++++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 38 ++++++++++--------- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 2 - drivers/gpu/drm/msm/msm_drv.c | 9 ++++- drivers/gpu/drm/msm/msm_drv.h | 6 ++- 9 files changed, 88 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index aaf5e2b..2d2551f 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -1,4 +1,5 @@ /* + * Copyright (c) 2014, The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark robdclark@gmail.com * @@ -17,6 +18,29 @@

#include "hdmi.h"

+int hdmi_set_encoder(struct platform_device *pdev, + struct drm_encoder *encoder) +{ + struct hdmi *hdmi = platform_get_drvdata(pdev); + + if (!encoder && !hdmi->encoder) { + pr_err("%s:wrong encoder status,encoder=%p,hdmi->encoder=%p\n", + __func__, encoder, hdmi->encoder); + return -EINVAL; + } + + /* Each connector has only one available encoder for now. */ + hdmi->connector->encoder_ids[0] = 0; + hdmi->connector->encoder = NULL; + if (encoder) { + drm_mode_connector_attach_encoder(hdmi->connector, encoder); + encoder->bridge = hdmi->bridge; + } + hdmi->encoder = encoder; + + return 0; +} + void hdmi_set_mode(struct hdmi *hdmi, bool power_on) { uint32_t ctrl = 0; @@ -54,10 +78,15 @@ static irqreturn_t hdmi_irq(int irq, void *dev_id) return IRQ_HANDLED; }

-void hdmi_destroy(struct kref *kref) +static void hdmi_destroy(struct platform_device *pdev) { - struct hdmi *hdmi = container_of(kref, struct hdmi, refcount); - struct hdmi_phy *phy = hdmi->phy; + struct hdmi *hdmi = platform_get_drvdata(pdev); + struct hdmi_phy *phy; + + if (!hdmi) + return; + + phy = hdmi->phy;

if (hdmi->config->shared_irq) msm_shared_irq_unregister(MSM_SUBSYS_HDMI); @@ -68,15 +97,14 @@ void hdmi_destroy(struct kref *kref) if (hdmi->i2c) hdmi_i2c_destroy(hdmi->i2c);

- platform_set_drvdata(hdmi->pdev, NULL); + platform_set_drvdata(pdev, NULL); }

/* initialize connector */ -struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder) +static int hdmi_init(struct platform_device *pdev, struct drm_device *dev) { struct hdmi *hdmi = NULL; struct msm_drm_private *priv = dev->dev_private; - struct platform_device *pdev = priv->hdmi_pdev; struct hdmi_platform_config *config; int i, ret;

@@ -94,12 +122,11 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder) goto fail; }

- kref_init(&hdmi->refcount); + platform_set_drvdata(pdev, hdmi);

hdmi->dev = dev; hdmi->pdev = pdev; hdmi->config = config; - hdmi->encoder = encoder;

hdmi_audio_infoframe_init(&hdmi->audio.infoframe);

@@ -233,14 +260,10 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder) } }

- encoder->bridge = hdmi->bridge; - priv->bridges[priv->num_bridges++] = hdmi->bridge; priv->connectors[priv->num_connectors++] = hdmi->connector;

- platform_set_drvdata(pdev, hdmi); - - return hdmi; + return 0;

fail: if (hdmi) { @@ -249,10 +272,10 @@ fail: hdmi->bridge->funcs->destroy(hdmi->bridge); if (hdmi->connector) hdmi->connector->funcs->destroy(hdmi->connector); - hdmi_destroy(&hdmi->refcount); + hdmi_destroy(pdev); }

- return ERR_PTR(ret); + return ret; }

/* @@ -289,6 +312,7 @@ static int get_gpio(struct device *dev, struct device_node *of_node, const char static int hdmi_bind(struct device *dev, struct device *master, void *data) { static struct hdmi_platform_config config = {}; + int ret; #ifdef CONFIG_OF struct device_node *of_node = dev->of_node;

@@ -379,6 +403,12 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data) } #endif dev->platform_data = &config; + + ret = hdmi_init(to_platform_device(dev), dev_get_drvdata(master)); + if (ret) { + dev_err(dev, "%s: hdmi_init fail, %d\n", __func__, ret); + return ret; + } set_hdmi_pdev(dev_get_drvdata(master), to_platform_device(dev)); return 0; } @@ -387,6 +417,7 @@ static void hdmi_unbind(struct device *dev, struct device *master, void *data) { set_hdmi_pdev(dev_get_drvdata(master), NULL); + hdmi_destroy(to_platform_device(dev)); }

static const struct component_ops hdmi_ops = { diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h index b981995..a78dd8d 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.h +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h @@ -38,8 +38,6 @@ struct hdmi_audio { };

struct hdmi { - struct kref refcount; - struct drm_device *dev; struct platform_device *pdev;

@@ -103,7 +101,6 @@ struct hdmi_platform_config { };

void hdmi_set_mode(struct hdmi *hdmi, bool power_on); -void hdmi_destroy(struct kref *kref);

static inline void hdmi_write(struct hdmi *hdmi, u32 reg, u32 data) { @@ -115,17 +112,6 @@ static inline u32 hdmi_read(struct hdmi *hdmi, u32 reg) return msm_readl(hdmi->mmio + reg); }

-static inline struct hdmi * hdmi_reference(struct hdmi *hdmi) -{ - kref_get(&hdmi->refcount); - return hdmi; -} - -static inline void hdmi_unreference(struct hdmi *hdmi) -{ - kref_put(&hdmi->refcount, hdmi_destroy); -} - /* * The phy appears to be different, for example between 8960 and 8x60, * so split the phy related functions out and load the correct one at diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index f6cf745..6902ad6 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -26,7 +26,6 @@ struct hdmi_bridge { static void hdmi_bridge_destroy(struct drm_bridge *bridge) { struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); - hdmi_unreference(hdmi_bridge->hdmi); drm_bridge_cleanup(bridge); kfree(hdmi_bridge); } @@ -218,7 +217,7 @@ struct drm_bridge *hdmi_bridge_init(struct hdmi *hdmi) goto fail; }

- hdmi_bridge->hdmi = hdmi_reference(hdmi); + hdmi_bridge->hdmi = hdmi;

bridge = &hdmi_bridge->base;

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index 4aca2a3..f5da877 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -330,8 +330,6 @@ static void hdmi_connector_destroy(struct drm_connector *connector) drm_connector_unregister(connector); drm_connector_cleanup(connector);

- hdmi_unreference(hdmi_connector->hdmi); - kfree(hdmi_connector); }

@@ -422,7 +420,7 @@ struct drm_connector *hdmi_connector_init(struct hdmi *hdmi) goto fail; }

- hdmi_connector->hdmi = hdmi_reference(hdmi); + hdmi_connector->hdmi = hdmi; INIT_WORK(&hdmi_connector->hpd_work, hotplug_work);

connector = &hdmi_connector->base; @@ -445,8 +443,6 @@ struct drm_connector *hdmi_connector_init(struct hdmi *hdmi) goto fail; }

- drm_mode_connector_attach_encoder(connector, hdmi->encoder); - return connector;

fail: diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c index 79d804e..b0b6dee 100644 --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c @@ -228,7 +228,6 @@ static int modeset_init(struct mdp4_kms *mdp4_kms) struct drm_encoder *encoder; struct drm_connector *connector; struct drm_panel *panel; - struct hdmi *hdmi; int ret;

/* construct non-private planes: */ @@ -326,15 +325,18 @@ static int modeset_init(struct mdp4_kms *mdp4_kms) priv->crtcs[priv->num_crtcs++] = crtc; priv->encoders[priv->num_encoders++] = encoder;

- hdmi = hdmi_init(dev, encoder); - if (IS_ERR(hdmi)) { - ret = PTR_ERR(hdmi); - dev_err(dev->dev, "failed to initialize HDMI: %d\n", ret); - goto fail; + ret = hdmi_set_encoder(priv->hdmi_pdev, encoder); + if (ret) { + dev_err(dev->dev, "failed to set encoder\n"); + goto fail1; }

return 0;

+fail1: + priv->encoders[--priv->num_encoders] = NULL; + if (encoder) + encoder->funcs->destroy(encoder); fail: return ret; } diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index 31a2c63..1bb3a28 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -302,14 +302,6 @@ static int modeset_init(struct mdp5_kms *mdp5_kms) priv->crtcs[priv->num_crtcs++] = crtc; }

- /* Construct encoder for HDMI: */ - encoder = mdp5_encoder_init(dev, 3, INTF_HDMI); - if (IS_ERR(encoder)) { - dev_err(dev->dev, "failed to construct encoder\n"); - ret = PTR_ERR(encoder); - goto fail; - } - /* NOTE: the vsync and error irq's are actually associated with * the INTF/encoder.. the easiest way to deal with this (ie. what * we do now) is assume a fixed relationship between crtc's and @@ -318,21 +310,33 @@ static int modeset_init(struct mdp5_kms *mdp5_kms) * care of error and vblank irq's that the crtc has registered, * and also update user-requested vblank_mask. */ - encoder->possible_crtcs = BIT(0); - mdp5_crtc_set_intf(priv->crtcs[0], 3, INTF_HDMI); + if (priv->hdmi_pdev) { + /* Construct encoder for HDMI: */ + encoder = mdp5_encoder_init(dev, 3, INTF_HDMI); + if (IS_ERR(encoder)) { + dev_err(dev->dev, "failed to construct encoder\n"); + ret = PTR_ERR(encoder); + goto fail; + }

- priv->encoders[priv->num_encoders++] = encoder; + encoder->possible_crtcs = BIT(0); + mdp5_crtc_set_intf(priv->crtcs[0], 3, INTF_HDMI);

- /* Construct bridge/connector for HDMI: */ - mdp5_kms->hdmi = hdmi_init(dev, encoder); - if (IS_ERR(mdp5_kms->hdmi)) { - ret = PTR_ERR(mdp5_kms->hdmi); - dev_err(dev->dev, "failed to initialize HDMI: %d\n", ret); - goto fail; + priv->encoders[priv->num_encoders++] = encoder; + + ret = hdmi_set_encoder(priv->hdmi_pdev, encoder); + if (ret) + goto fail1; }

return 0;

+fail1: + if (priv->hdmi_pdev) { + priv->encoders[--priv->num_encoders] = NULL; + if (encoder) + encoder->funcs->destroy(encoder); + } fail: return ret; } diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h index 5bf340d..c91101d 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h @@ -71,8 +71,6 @@ struct mdp5_kms { struct clk *lut_clk; struct clk *vsync_clk;

- struct hdmi *hdmi; - struct mdp_irq error_handler; }; #define to_mdp5_kms(x) container_of(x, struct mdp5_kms, base) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index b67ef59..c27cb9a 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -147,7 +147,10 @@ static int msm_unload(struct drm_device *dev) priv->vram.paddr, &attrs); }

- component_unbind_all(dev->dev, dev); + if (priv->component_bound) { + component_unbind_all(dev->dev, dev); + priv->component_bound = false; + }

dev->dev_private = NULL;

@@ -237,7 +240,9 @@ static int msm_load(struct drm_device *dev, unsigned long flags) /* Bind all our sub-components: */ ret = component_bind_all(dev->dev, dev); if (ret) - return ret; + goto fail; + + priv->component_bound = true;

switch (get_mdp_ver(pdev)) { case 4: diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 718ac55..76cac63 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -125,6 +125,8 @@ struct msm_drm_private { */ struct drm_mm mm; } vram; + + bool component_bound; };

/* For mdp5 only */ @@ -219,10 +221,10 @@ struct drm_framebuffer *msm_framebuffer_create(struct drm_device *dev,

struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev);

-struct hdmi; -struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder); void __init hdmi_register(void); void __exit hdmi_unregister(void); +int hdmi_set_encoder(struct platform_device *pdev, + struct drm_encoder *encoder);

#ifdef CONFIG_DEBUG_FS void msm_gem_describe(struct drm_gem_object *obj, struct seq_file *m);