All the sub-systems in mdss share the same irq. This change provides the sub-systems with the interfaces to register/unregister their own irq handlers.
With this change, struct mdp5_kms does not have to keep the hdmi or edp context.
Signed-off-by: Hai Li hali@codeaurora.org --- drivers/gpu/drm/msm/hdmi/hdmi.c | 12 +++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c | 107 ++++++++++++++++++++++++++++++-- drivers/gpu/drm/msm/msm_drv.h | 19 +++++- 3 files changed, 130 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 9d00dcb..aaf5e2b 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -39,7 +39,7 @@ void hdmi_set_mode(struct hdmi *hdmi, bool power_on) power_on ? "Enable" : "Disable", ctrl); }
-irqreturn_t hdmi_irq(int irq, void *dev_id) +static irqreturn_t hdmi_irq(int irq, void *dev_id) { struct hdmi *hdmi = dev_id;
@@ -59,6 +59,9 @@ void hdmi_destroy(struct kref *kref) struct hdmi *hdmi = container_of(kref, struct hdmi, refcount); struct hdmi_phy *phy = hdmi->phy;
+ if (hdmi->config->shared_irq) + msm_shared_irq_unregister(MSM_SUBSYS_HDMI); + if (phy) phy->funcs->destroy(phy);
@@ -221,6 +224,13 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder) hdmi->irq, ret); goto fail; } + } else { + ret = msm_shared_irq_register(MSM_SUBSYS_HDMI, hdmi_irq, hdmi); + if (ret < 0) { + dev_err(dev->dev, "failed to register shared IRQ: %d\n", + ret); + goto fail; + } }
encoder->bridge = hdmi->bridge; diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c index f2b985b..2973c1c 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.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 * @@ -19,6 +20,75 @@ #include "msm_drv.h" #include "mdp5_kms.h"
+struct msm_subsys_shared_irq { + u32 mask; + u32 count; + + /* Filled by sub system */ + irqreturn_t (*handler)(int irq, void *dev_id); + void *data; +}; + +static struct msm_subsys_shared_irq msm_shared_irqs[MSM_SUBSYS_COUNT] = { + [MSM_SUBSYS_MDP] = {.mask = MDP5_HW_INTR_STATUS_INTR_MDP, + .count = 0}, + [MSM_SUBSYS_DSI_0] = {.mask = MDP5_HW_INTR_STATUS_INTR_DSI0, + .count = 0}, + [MSM_SUBSYS_DSI_1] = {.mask = MDP5_HW_INTR_STATUS_INTR_DSI1, + .count = 0}, + [MSM_SUBSYS_HDMI] = {.mask = MDP5_HW_INTR_STATUS_INTR_HDMI, + .count = 0}, + [MSM_SUBSYS_EDP] = {.mask = MDP5_HW_INTR_STATUS_INTR_EDP, + .count = 0}, +}; + +static irqreturn_t mdp5_irq_mdp(int irq, void *dev_id); + +int msm_shared_irq_register(enum msm_sub_system sys_id, + irqreturn_t (*handler)(int irq, void *dev_id), void *data) +{ + if (sys_id >= MSM_SUBSYS_COUNT) { + DRM_ERROR("Invalid sys_id %d", sys_id); + return -EINVAL; + } + + if (msm_shared_irqs[sys_id].handler != NULL) { + DRM_ERROR("sys %d irq already registered", sys_id); + return -EBUSY; + } + + msm_shared_irqs[sys_id].data = data; + msm_shared_irqs[sys_id].handler = handler; + + return 0; +} + +/* + * This function should be called after the interrupt + * on the sub-system is disabled. + */ +int msm_shared_irq_unregister(enum msm_sub_system sys_id) +{ + if (sys_id >= MSM_SUBSYS_COUNT) { + DRM_ERROR("Invalid sys_id %d", sys_id); + return -EINVAL; + } + + msm_shared_irqs[sys_id].handler = NULL; + msm_shared_irqs[sys_id].data = NULL; + + /* + * Make sure irq_handler and data is invalid. + * Then we only need to wait until the last pending interrupt is done. + */ + wmb(); + + while (msm_shared_irqs[sys_id].count & 0x1) + usleep_range(100, 1000); + + return 0; +} + void mdp5_set_irqmask(struct mdp_kms *mdp_kms, uint32_t irqmask) { mdp5_write(to_mdp5_kms(mdp_kms), REG_MDP5_INTR_EN, irqmask); @@ -47,6 +117,9 @@ int mdp5_irq_postinstall(struct msm_kms *kms) MDP5_IRQ_INTF2_UNDER_RUN | MDP5_IRQ_INTF3_UNDER_RUN;
+ /* Register mdp irq to mdss */ + msm_shared_irq_register(MSM_SUBSYS_MDP, mdp5_irq_mdp, mdp_kms); + mdp_irq_register(mdp_kms, error_handler);
return 0; @@ -56,10 +129,15 @@ void mdp5_irq_uninstall(struct msm_kms *kms) { struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); mdp5_write(mdp5_kms, REG_MDP5_INTR_EN, 0x00000000); + + /* Make sure interrupt is disabled before remove irq. */ + wmb(); + msm_shared_irq_unregister(MSM_SUBSYS_MDP); }
-static void mdp5_irq_mdp(struct mdp_kms *mdp_kms) +static irqreturn_t mdp5_irq_mdp(int irq, void *dev_id) { + struct mdp_kms *mdp_kms = (struct mdp_kms *)dev_id; struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms); struct drm_device *dev = mdp5_kms->dev; struct msm_drm_private *priv = dev->dev_private; @@ -76,23 +154,40 @@ static void mdp5_irq_mdp(struct mdp_kms *mdp_kms) for (id = 0; id < priv->num_crtcs; id++) if (status & mdp5_crtc_vblank(priv->crtcs[id])) drm_handle_vblank(dev, id); + + return IRQ_HANDLED; }
irqreturn_t mdp5_irq(struct msm_kms *kms) { struct mdp_kms *mdp_kms = to_mdp_kms(kms); struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms); + struct msm_subsys_shared_irq *irq; uint32_t intr; + int i;
intr = mdp5_read(mdp5_kms, REG_MDP5_HW_INTR_STATUS);
VERB("intr=%08x", intr);
- if (intr & MDP5_HW_INTR_STATUS_INTR_MDP) - mdp5_irq_mdp(mdp_kms); - - if (intr & MDP5_HW_INTR_STATUS_INTR_HDMI) - hdmi_irq(0, mdp5_kms->hdmi); + for (i = 0; i < MSM_SUBSYS_COUNT; i++) { + irq = &msm_shared_irqs[i]; + if (intr & irq->mask) { + irq->count++; + + /* + * These 2 wmb() ensure count is odd number + * during handler is running. + */ + wmb(); + if ((irq->handler != NULL) && (irq->data != NULL)) + irq->handler(0, irq->data); + + /* Make sure count increments after handler is done */ + wmb(); + irq->count++; + } + }
return IRQ_HANDLED; } diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 67f9d0a..718ac55 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -127,6 +127,16 @@ struct msm_drm_private { } vram; };
+/* For mdp5 only */ +enum msm_sub_system { + MSM_SUBSYS_MDP = 0, + MSM_SUBSYS_DSI_0, + MSM_SUBSYS_DSI_1, + MSM_SUBSYS_HDMI, + MSM_SUBSYS_EDP, + MSM_SUBSYS_COUNT +}; + struct msm_format { uint32_t pixel_format; }; @@ -145,6 +155,14 @@ void __msm_fence_worker(struct work_struct *work); (_cb)->func = _func; \ } while (0)
+/* + * For mdp5 only, callers should call these 2 functions + * only if the irqs are shared with others. + */ +int msm_shared_irq_register(enum msm_sub_system sys_id, + irqreturn_t (*handler)(int irq, void *dev_id), void *data); +int msm_shared_irq_unregister(enum msm_sub_system sys_id); + int msm_register_mmu(struct drm_device *dev, struct msm_mmu *mmu);
int msm_wait_fence_interruptable(struct drm_device *dev, uint32_t fence, @@ -203,7 +221,6 @@ 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); -irqreturn_t hdmi_irq(int irq, void *dev_id); void __init hdmi_register(void); void __exit hdmi_unregister(void);
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);
On Fri, Nov 14, 2014 at 5:42 PM, Hai Li hali@codeaurora.org wrote:
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.
this looks like it is going to conflict somewhat with:
http://cgit.freedesktop.org/~robclark/linux/commit/?h=msm-next&id=3b3a29...
I had already refactored the hdmi init to better handle probe-defer (and move all the devm_get's into hdmi_bind()). It is somewhat similar to what you are doing here.. hdmi_init() is called at hdmi_bind() time, and then hdmi_modeset_init() is called at toplevel drm device modeset_init() time to construct the kms modeset object (connector/bridge) and connect up with encoder.
I didn't remove the kref yet, although it probably could/should be removed. It doesn't really protect us if the hdmi device is removed before the toplevel drm device (since all the devm resources would be dropped). But AFAIU the component framework stuff should protect us from that. The kref is just left over from before component framework because I forgot to remove it ;-)
BR, -R
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); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
On Fri, Nov 14, 2014 at 5:42 PM, Hai Li hali@codeaurora.org wrote:
All the sub-systems in mdss share the same irq. This change provides the sub-systems with the interfaces to register/unregister their own irq handlers.
With this change, struct mdp5_kms does not have to keep the hdmi or edp context.
So, I think the point of this is to better deal w/ different hw variants which do or do-not have hdmi, edp, dsi, etc..
But, just playing devil's advocate here, it seems like it would be simpler to instead just do something like:
if (priv->hdmi && (intr & MDP5_HW_INTR_STATUS_INTR_HDMI)) hdmi_irq(0, priv->hdmi);
if (priv->edp && (intr & MDP5_HW_INTR_STATUS_INTR_EDP)) edp_irq(0, priv->edp);
... etc ...
It is a little less elegant. But it is also less lines of code. I guess there will be plenty of necessary complexity as we add support for all mdp5 features. So avoiding some unnecessary complexity might be a good thing in the long run.
If we really did want to make it more dynamic, we could always extend 'struct mdp_irq' to take both an irq mask and an initiator id, I suppose. I'm not sure if that is overkill. AFAICT we really only have 5 different subsystems to dispatch to (mdp5 itself, and dsi0/dsi1/hdmi/edp), so simply adding some if-not-null checks in mdp5_irq() seems pretty reasonable to me.
BR, -R
Signed-off-by: Hai Li hali@codeaurora.org
drivers/gpu/drm/msm/hdmi/hdmi.c | 12 +++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c | 107 ++++++++++++++++++++++++++++++-- drivers/gpu/drm/msm/msm_drv.h | 19 +++++- 3 files changed, 130 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 9d00dcb..aaf5e2b 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -39,7 +39,7 @@ void hdmi_set_mode(struct hdmi *hdmi, bool power_on) power_on ? "Enable" : "Disable", ctrl); }
-irqreturn_t hdmi_irq(int irq, void *dev_id) +static irqreturn_t hdmi_irq(int irq, void *dev_id) { struct hdmi *hdmi = dev_id;
@@ -59,6 +59,9 @@ void hdmi_destroy(struct kref *kref) struct hdmi *hdmi = container_of(kref, struct hdmi, refcount); struct hdmi_phy *phy = hdmi->phy;
if (hdmi->config->shared_irq)msm_shared_irq_unregister(MSM_SUBSYS_HDMI);if (phy) phy->funcs->destroy(phy);@@ -221,6 +224,13 @@ struct hdmi *hdmi_init(struct drm_device *dev, struct drm_encoder *encoder) hdmi->irq, ret); goto fail; }
} else {ret = msm_shared_irq_register(MSM_SUBSYS_HDMI, hdmi_irq, hdmi);if (ret < 0) {dev_err(dev->dev, "failed to register shared IRQ: %d\n",ret);goto fail;} } encoder->bridge = hdmi->bridge;diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c index f2b985b..2973c1c 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.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
@@ -19,6 +20,75 @@ #include "msm_drv.h" #include "mdp5_kms.h"
+struct msm_subsys_shared_irq {
u32 mask;u32 count;/* Filled by sub system */irqreturn_t (*handler)(int irq, void *dev_id);void *data;+};
+static struct msm_subsys_shared_irq msm_shared_irqs[MSM_SUBSYS_COUNT] = {
[MSM_SUBSYS_MDP] = {.mask = MDP5_HW_INTR_STATUS_INTR_MDP,.count = 0},[MSM_SUBSYS_DSI_0] = {.mask = MDP5_HW_INTR_STATUS_INTR_DSI0,.count = 0},[MSM_SUBSYS_DSI_1] = {.mask = MDP5_HW_INTR_STATUS_INTR_DSI1,.count = 0},[MSM_SUBSYS_HDMI] = {.mask = MDP5_HW_INTR_STATUS_INTR_HDMI,.count = 0},[MSM_SUBSYS_EDP] = {.mask = MDP5_HW_INTR_STATUS_INTR_EDP,.count = 0},+};
+static irqreturn_t mdp5_irq_mdp(int irq, void *dev_id);
+int msm_shared_irq_register(enum msm_sub_system sys_id,
irqreturn_t (*handler)(int irq, void *dev_id), void *data)+{
if (sys_id >= MSM_SUBSYS_COUNT) {DRM_ERROR("Invalid sys_id %d", sys_id);return -EINVAL;}if (msm_shared_irqs[sys_id].handler != NULL) {DRM_ERROR("sys %d irq already registered", sys_id);return -EBUSY;}msm_shared_irqs[sys_id].data = data;msm_shared_irqs[sys_id].handler = handler;return 0;+}
+/*
- This function should be called after the interrupt
- on the sub-system is disabled.
- */
+int msm_shared_irq_unregister(enum msm_sub_system sys_id) +{
if (sys_id >= MSM_SUBSYS_COUNT) {DRM_ERROR("Invalid sys_id %d", sys_id);return -EINVAL;}msm_shared_irqs[sys_id].handler = NULL;msm_shared_irqs[sys_id].data = NULL;/** Make sure irq_handler and data is invalid.* Then we only need to wait until the last pending interrupt is done.*/wmb();while (msm_shared_irqs[sys_id].count & 0x1)usleep_range(100, 1000);return 0;+}
void mdp5_set_irqmask(struct mdp_kms *mdp_kms, uint32_t irqmask) { mdp5_write(to_mdp5_kms(mdp_kms), REG_MDP5_INTR_EN, irqmask); @@ -47,6 +117,9 @@ int mdp5_irq_postinstall(struct msm_kms *kms) MDP5_IRQ_INTF2_UNDER_RUN | MDP5_IRQ_INTF3_UNDER_RUN;
/* Register mdp irq to mdss */msm_shared_irq_register(MSM_SUBSYS_MDP, mdp5_irq_mdp, mdp_kms);mdp_irq_register(mdp_kms, error_handler); return 0;@@ -56,10 +129,15 @@ void mdp5_irq_uninstall(struct msm_kms *kms) { struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); mdp5_write(mdp5_kms, REG_MDP5_INTR_EN, 0x00000000);
/* Make sure interrupt is disabled before remove irq. */wmb();msm_shared_irq_unregister(MSM_SUBSYS_MDP);}
-static void mdp5_irq_mdp(struct mdp_kms *mdp_kms) +static irqreturn_t mdp5_irq_mdp(int irq, void *dev_id) {
struct mdp_kms *mdp_kms = (struct mdp_kms *)dev_id; struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms); struct drm_device *dev = mdp5_kms->dev; struct msm_drm_private *priv = dev->dev_private;@@ -76,23 +154,40 @@ static void mdp5_irq_mdp(struct mdp_kms *mdp_kms) for (id = 0; id < priv->num_crtcs; id++) if (status & mdp5_crtc_vblank(priv->crtcs[id])) drm_handle_vblank(dev, id);
return IRQ_HANDLED;}
irqreturn_t mdp5_irq(struct msm_kms *kms) { struct mdp_kms *mdp_kms = to_mdp_kms(kms); struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms);
struct msm_subsys_shared_irq *irq; uint32_t intr;int i; intr = mdp5_read(mdp5_kms, REG_MDP5_HW_INTR_STATUS); VERB("intr=%08x", intr);
if (intr & MDP5_HW_INTR_STATUS_INTR_MDP)mdp5_irq_mdp(mdp_kms);if (intr & MDP5_HW_INTR_STATUS_INTR_HDMI)hdmi_irq(0, mdp5_kms->hdmi);
for (i = 0; i < MSM_SUBSYS_COUNT; i++) {irq = &msm_shared_irqs[i];if (intr & irq->mask) {irq->count++;/** These 2 wmb() ensure count is odd number* during handler is running.*/wmb();if ((irq->handler != NULL) && (irq->data != NULL))irq->handler(0, irq->data);/* Make sure count increments after handler is done */wmb();irq->count++;}} return IRQ_HANDLED;} diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 67f9d0a..718ac55 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -127,6 +127,16 @@ struct msm_drm_private { } vram; };
+/* For mdp5 only */ +enum msm_sub_system {
MSM_SUBSYS_MDP = 0,MSM_SUBSYS_DSI_0,MSM_SUBSYS_DSI_1,MSM_SUBSYS_HDMI,MSM_SUBSYS_EDP,MSM_SUBSYS_COUNT+};
struct msm_format { uint32_t pixel_format; }; @@ -145,6 +155,14 @@ void __msm_fence_worker(struct work_struct *work); (_cb)->func = _func; \ } while (0)
+/*
- For mdp5 only, callers should call these 2 functions
- only if the irqs are shared with others.
- */
+int msm_shared_irq_register(enum msm_sub_system sys_id,
irqreturn_t (*handler)(int irq, void *dev_id), void *data);+int msm_shared_irq_unregister(enum msm_sub_system sys_id);
int msm_register_mmu(struct drm_device *dev, struct msm_mmu *mmu);
int msm_wait_fence_interruptable(struct drm_device *dev, uint32_t fence, @@ -203,7 +221,6 @@ 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); -irqreturn_t hdmi_irq(int irq, void *dev_id); void __init hdmi_register(void); void __exit hdmi_unregister(void);
-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
On Sun, Nov 16, 2014 at 09:18:47AM -0500, Rob Clark wrote:
On Fri, Nov 14, 2014 at 5:42 PM, Hai Li hali@codeaurora.org wrote:
All the sub-systems in mdss share the same irq. This change provides the sub-systems with the interfaces to register/unregister their own irq handlers.
With this change, struct mdp5_kms does not have to keep the hdmi or edp context.
So, I think the point of this is to better deal w/ different hw variants which do or do-not have hdmi, edp, dsi, etc..
But, just playing devil's advocate here, it seems like it would be simpler to instead just do something like:
if (priv->hdmi && (intr & MDP5_HW_INTR_STATUS_INTR_HDMI)) hdmi_irq(0, priv->hdmi); if (priv->edp && (intr & MDP5_HW_INTR_STATUS_INTR_EDP)) edp_irq(0, priv->edp); ... etc ...It is a little less elegant. But it is also less lines of code. I guess there will be plenty of necessary complexity as we add support for all mdp5 features. So avoiding some unnecessary complexity might be a good thing in the long run.
If we really did want to make it more dynamic, we could always extend 'struct mdp_irq' to take both an irq mask and an initiator id, I suppose. I'm not sure if that is overkill. AFAICT we really only have 5 different subsystems to dispatch to (mdp5 itself, and dsi0/dsi1/hdmi/edp), so simply adding some if-not-null checks in mdp5_irq() seems pretty reasonable to me.
To me this just seems like a regular interrupt multiplexer, so implementing it as an irq_chip would be the most appropriate. That way you get all the goodness of a well-tested code base for free and you can simply pass in the request_{threaded_,}irq()'s dev parameter.
Thierry
On Thu, Nov 20, 2014 at 3:57 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Sun, Nov 16, 2014 at 09:18:47AM -0500, Rob Clark wrote:
On Fri, Nov 14, 2014 at 5:42 PM, Hai Li hali@codeaurora.org wrote:
All the sub-systems in mdss share the same irq. This change provides the sub-systems with the interfaces to register/unregister their own irq handlers.
With this change, struct mdp5_kms does not have to keep the hdmi or edp context.
So, I think the point of this is to better deal w/ different hw variants which do or do-not have hdmi, edp, dsi, etc..
But, just playing devil's advocate here, it seems like it would be simpler to instead just do something like:
if (priv->hdmi && (intr & MDP5_HW_INTR_STATUS_INTR_HDMI)) hdmi_irq(0, priv->hdmi); if (priv->edp && (intr & MDP5_HW_INTR_STATUS_INTR_EDP)) edp_irq(0, priv->edp); ... etc ...It is a little less elegant. But it is also less lines of code. I guess there will be plenty of necessary complexity as we add support for all mdp5 features. So avoiding some unnecessary complexity might be a good thing in the long run.
If we really did want to make it more dynamic, we could always extend 'struct mdp_irq' to take both an irq mask and an initiator id, I suppose. I'm not sure if that is overkill. AFAICT we really only have 5 different subsystems to dispatch to (mdp5 itself, and dsi0/dsi1/hdmi/edp), so simply adding some if-not-null checks in mdp5_irq() seems pretty reasonable to me.
To me this just seems like a regular interrupt multiplexer, so implementing it as an irq_chip would be the most appropriate. That way you get all the goodness of a well-tested code base for free and you can simply pass in the request_{threaded_,}irq()'s dev parameter.
yup, that is what I did here:
http://cgit.freedesktop.org/~robclark/linux/commit/?h=msm-next&id=d9a709...
there is a bit of awkwardness related to threaded handlers.. for now I don't need threaded handlers so the solution was to not use them
Thierry
dri-devel@lists.freedesktop.org
-
Hai Li -
Rob Clark -
Thierry Reding