The RFC concerns a code restructuring of the HDMI module, which mainly has two purposes:
1. architectural HDMI registers are divided in two parts: HDMI core and HDMI PHY. The HDMI Core registers are common to all chipsets sharing the MDP version (MDP5 here). On the other hand, HDMI PHY registers depend on the interface and process (28nm, 20nm, ...) being used for the same chipsets. Some chipsets (eg: msm8x94) don't require us to access HDMI PHY registers.
From a software architecture point of view, it would make sense that all HDMI
PHY registers should only be accessed by (PHY) hdmi_phy_*.c files, while all HDMI Core registers are read/written by the (core) hdmi.c and hdmi_connector.c files.
2. readability Once we add msm8x94, which does not program HDMI PHY registers, it will be easier to handle the HDMI core register programming because a hdmi_phy_*.c file won't be created for this chipset. Instead of creating such a stub file or some kind of workaround, it only makes sense to bring the HDMI core register programming where it is supposed to be.
To summarize, this re-structuring is to improve the code architecture, facilitate the introduction of msm8x94 support and won't change the current functionality.
Stephane Viau (2): [RFC] drm/msm/hdmi: remove ->reset() from HDMI PHY drm/msm: Add support for msm8x94
drivers/gpu/drm/msm/hdmi/hdmi.c | 30 ++++++---- drivers/gpu/drm/msm/hdmi/hdmi.h | 1 - drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 8 ++- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 94 +++++++++++++++++++++++++------ drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c | 52 ----------------- drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c | 32 ----------- drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c | 57 ------------------- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 73 +++++++++++++++++++++++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 8 ++- 9 files changed, 179 insertions(+), 176 deletions(-)
->reset() currently only accesses HDMI core registers, and yet it is located in hdmi_phy*. Since no PHY registers are being accessed during ->reset(), it would be better to bring that function in hdmi core module where HDMI core registers are usually being accessed.
This will also help for msm8x94 for which no PHY registers accesses are done (->phy_init == NULL) but the HDMI PHY reset from HDMI core still needs to be done.
Note: SW_RESET_PLL bit is not written in hdmi_phy_8x60_reset(); this write should not affect anything if the corresponding field is not writable.
Signed-off-by: Stephane Viau sviau@codeaurora.org --- drivers/gpu/drm/msm/hdmi/hdmi.h | 1 - drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 51 ++++++++++++++++++++++++++- drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c | 52 ---------------------------- drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c | 32 ----------------- drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c | 57 ------------------------------- 5 files changed, 50 insertions(+), 143 deletions(-)
<snip>
This change adds the MDP and HDMI support for msm8x94. Note that HDMI PHY registers are not being accessed anymore from the driver.
Signed-off-by: Stephane Viau sviau@codeaurora.org --- drivers/gpu/drm/msm/hdmi/hdmi.c | 30 ++++++++----- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 8 +++- drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 43 +++++++++++------- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 73 ++++++++++++++++++++++++++++++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 8 ++-- 5 files changed, 129 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 8145362..4d73a69 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -89,16 +89,15 @@ static struct hdmi *hdmi_init(struct platform_device *pdev) hdmi->config = config;
/* not sure about which phy maps to which msm.. probably I miss some */ - if (config->phy_init) + if (config->phy_init) { hdmi->phy = config->phy_init(hdmi); - else - hdmi->phy = ERR_PTR(-ENXIO);
- if (IS_ERR(hdmi->phy)) { - ret = PTR_ERR(hdmi->phy); - dev_err(&pdev->dev, "failed to load phy: %d\n", ret); - hdmi->phy = NULL; - goto fail; + if (IS_ERR(hdmi->phy)) { + ret = PTR_ERR(hdmi->phy); + dev_err(&pdev->dev, "failed to load phy: %d\n", ret); + hdmi->phy = NULL; + goto fail; + } }
hdmi->mmio = msm_ioremap(pdev, config->mmio_name, "HDMI"); @@ -330,7 +329,19 @@ static struct hdmi_platform_config hdmi_tx_8084_config = { .hpd_freq = hpd_clk_freq_8x74, };
+static const char *hpd_reg_names_8x94[] = {}; + +static struct hdmi_platform_config hdmi_tx_8x94_config = { + .phy_init = NULL, /* nothing to do for this HDMI PHY 20nm */ + HDMI_CFG(pwr_reg, 8x74), + HDMI_CFG(hpd_reg, 8x94), + HDMI_CFG(pwr_clk, 8x74), + HDMI_CFG(hpd_clk, 8x74), + .hpd_freq = hpd_clk_freq_8x74, +}; + static const struct of_device_id dt_match[] = { + { .compatible = "qcom,hdmi-tx-8x94", .data = &hdmi_tx_8x94_config }, { .compatible = "qcom,hdmi-tx-8084", .data = &hdmi_tx_8084_config }, { .compatible = "qcom,hdmi-tx-8074", .data = &hdmi_tx_8074_config }, { .compatible = "qcom,hdmi-tx-8960", .data = &hdmi_tx_8960_config }, @@ -347,8 +358,7 @@ static int get_gpio(struct device *dev, struct device_node *of_node, const char snprintf(name2, sizeof(name2), "%s-gpio", name); gpio = of_get_named_gpio(of_node, name2, 0); if (gpio < 0) { - dev_err(dev, "failed to get gpio: %s (%d)\n", - name, gpio); + DBG("failed to get gpio: %s (%d)", name, gpio); gpio = -1; } } diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index a7a1d82..00cec41 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -100,7 +100,9 @@ static void hdmi_bridge_pre_enable(struct drm_bridge *bridge) hdmi_audio_update(hdmi); }
- phy->funcs->powerup(phy, hdmi->pixclock); + if (phy) + phy->funcs->powerup(phy, hdmi->pixclock); + hdmi_set_mode(hdmi, true); }
@@ -120,7 +122,9 @@ static void hdmi_bridge_post_disable(struct drm_bridge *bridge)
DBG("power down"); hdmi_set_mode(hdmi, false); - phy->funcs->powerdown(phy); + + if (phy) + phy->funcs->powerdown(phy);
if (hdmi->power_on) { power_off(bridge); diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c index ece572d..cdc818c 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c @@ -84,21 +84,25 @@ static int gpio_config(struct hdmi *hdmi, bool on) int ret;
if (on) { - ret = gpio_request(config->ddc_clk_gpio, "HDMI_DDC_CLK"); - if (ret) { - dev_err(dev, "'%s'(%d) gpio_request failed: %d\n", - "HDMI_DDC_CLK", config->ddc_clk_gpio, ret); - goto error1; + if (config->ddc_clk_gpio != -1) { + ret = gpio_request(config->ddc_clk_gpio, "HDMI_DDC_CLK"); + if (ret) { + dev_err(dev, "'%s'(%d) gpio_request failed: %d\n", + "HDMI_DDC_CLK", config->ddc_clk_gpio, ret); + goto error1; + } + gpio_set_value_cansleep(config->ddc_clk_gpio, 1); } - gpio_set_value_cansleep(config->ddc_clk_gpio, 1);
- ret = gpio_request(config->ddc_data_gpio, "HDMI_DDC_DATA"); - if (ret) { - dev_err(dev, "'%s'(%d) gpio_request failed: %d\n", - "HDMI_DDC_DATA", config->ddc_data_gpio, ret); - goto error2; + if (config->ddc_data_gpio != -1) { + ret = gpio_request(config->ddc_data_gpio, "HDMI_DDC_DATA"); + if (ret) { + dev_err(dev, "'%s'(%d) gpio_request failed: %d\n", + "HDMI_DDC_DATA", config->ddc_data_gpio, ret); + goto error2; + } + gpio_set_value_cansleep(config->ddc_data_gpio, 1); } - gpio_set_value_cansleep(config->ddc_data_gpio, 1);
ret = gpio_request(config->hpd_gpio, "HDMI_HPD"); if (ret) { @@ -143,8 +147,12 @@ static int gpio_config(struct hdmi *hdmi, bool on) } DBG("gpio on"); } else { - gpio_free(config->ddc_clk_gpio); - gpio_free(config->ddc_data_gpio); + if (config->ddc_clk_gpio != -1) + gpio_free(config->ddc_clk_gpio); + + if (config->ddc_data_gpio != -1) + gpio_free(config->ddc_data_gpio); + gpio_free(config->hpd_gpio);
if (config->mux_en_gpio != -1) { @@ -175,9 +183,11 @@ error5: error4: gpio_free(config->hpd_gpio); error3: - gpio_free(config->ddc_data_gpio); + if (config->ddc_data_gpio != -1) + gpio_free(config->ddc_data_gpio); error2: - gpio_free(config->ddc_clk_gpio); + if (config->ddc_clk_gpio != -1) + gpio_free(config->ddc_clk_gpio); error1: return ret; } @@ -187,7 +197,6 @@ static int hpd_enable(struct hdmi_connector *hdmi_connector) struct hdmi *hdmi = hdmi_connector->hdmi; const struct hdmi_platform_config *config = hdmi->config; struct device *dev = &hdmi->pdev->dev; - struct hdmi_phy *phy = hdmi->phy; uint32_t hpd_ctrl; int i, ret;
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c index 9ff25a3..b064c5f 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c @@ -203,14 +203,85 @@ const struct mdp5_cfg_hw msm8x16_config = { .max_clk = 320000000, };
+const struct mdp5_cfg_hw msm8x94_config = { + .name = "msm8x94", + .mdp = { + .count = 1, + .base = { 0x01000 }, + }, + .smp = { + .mmb_count = 44, + .mmb_size = 8192, + .clients = { + [SSPP_VIG0] = 1, [SSPP_VIG1] = 4, + [SSPP_VIG2] = 7, [SSPP_VIG3] = 19, + [SSPP_DMA0] = 10, [SSPP_DMA1] = 13, + [SSPP_RGB0] = 16, [SSPP_RGB1] = 17, + [SSPP_RGB2] = 18, [SSPP_RGB3] = 22, + }, + .reserved_state[0] = GENMASK(23, 0), /* first 24 MMBs */ + .reserved = { + [ 1] = 1, [ 4] = 1, [ 7] = 1, [19] = 1, + [16] = 5, [17] = 5, [18] = 5, [22] = 5, + }, + }, + .ctl = { + .count = 5, + .base = { 0x02000, 0x02200, 0x02400, 0x02600, 0x02800 }, + .flush_hw_mask = 0xf0ffffff, + }, + .pipe_vig = { + .count = 4, + .base = { 0x05000, 0x07000, 0x09000, 0x0b000 }, + /* TODO: add decimation bit */ + }, + .pipe_rgb = { + .count = 4, + .base = { 0x15000, 0x17000, 0x19000, 0x1b000 }, + /* TODO: add decimation bit */ + }, + .pipe_dma = { + .count = 2, + .base = { 0x25000, 0x27000 }, + }, + .lm = { + .count = 6, + .base = { 0x45000, 0x46000, 0x47000, 0x48000, 0x49000, 0x4a000 }, + .nb_stages = 8, + }, + .dspp = { + .count = 4, + .base = { 0x55000, 0x57000, 0x59000, 0x5b000 }, + + }, + .ad = { + .count = 3, + .base = { 0x79000, 0x79800, 0x7a000 }, + }, + .pp = { + .count = 4, + .base = { 0x71000, 0x71800, 0x72000, 0x72800 }, + }, + .intf = { + .base = { 0x6b000, 0x6b800, 0x6c000, 0x6c800, 0x6d000 }, + .connect = { + [0] = INTF_DISABLED, + [1] = INTF_DSI, + [2] = INTF_DSI, + [3] = INTF_HDMI, + }, + }, + .max_clk = 320000000, +}; + static const struct mdp5_cfg_handler cfg_handlers[] = { { .revision = 0, .config = { .hw = &msm8x74_config } }, { .revision = 2, .config = { .hw = &msm8x74_config } }, { .revision = 3, .config = { .hw = &apq8084_config } }, { .revision = 6, .config = { .hw = &msm8x16_config } }, + { .revision = 9, .config = { .hw = &msm8x94_config } }, };
- static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev);
const struct mdp5_cfg_hw *mdp5_cfg_get_hw_config(struct mdp5_cfg_handler *cfg_handler) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index f6c5896..97226a1 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -159,7 +159,8 @@ int mdp5_disable(struct mdp5_kms *mdp5_kms) clk_disable_unprepare(mdp5_kms->ahb_clk); clk_disable_unprepare(mdp5_kms->axi_clk); clk_disable_unprepare(mdp5_kms->core_clk); - clk_disable_unprepare(mdp5_kms->lut_clk); + if (mdp5_kms->lut_clk) + clk_disable_unprepare(mdp5_kms->lut_clk);
return 0; } @@ -171,7 +172,8 @@ int mdp5_enable(struct mdp5_kms *mdp5_kms) clk_prepare_enable(mdp5_kms->ahb_clk); clk_prepare_enable(mdp5_kms->axi_clk); clk_prepare_enable(mdp5_kms->core_clk); - clk_prepare_enable(mdp5_kms->lut_clk); + if (mdp5_kms->lut_clk) + clk_prepare_enable(mdp5_kms->lut_clk);
return 0; } @@ -471,7 +473,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) goto fail; ret = get_clk(pdev, &mdp5_kms->lut_clk, "lut_clk"); if (ret) - goto fail; + DBG("failed to get (optional) lut_clk clock"); ret = get_clk(pdev, &mdp5_kms->vsync_clk, "vsync_clk"); if (ret) goto fail;
dri-devel@lists.freedesktop.org
-
Stephane Viau