On Mon, Jul 9, 2018 at 1:31 PM, Sean Paul seanpaul@chromium.org wrote:

From: Jeykumar Sankaran jsanka@codeaurora.org

Qualcomm Snapdragon chipsets uses compressed format to optimize BW across multiple IP's. This change adds needed modifier support in drm for a simple 4x4 tile based compressed variants of base formats.

Signed-off-by: Jeykumar Sankaran jsanka@codeaurora.org Signed-off-by: Sean Paul seanpaul@chromium.org

---

include/uapi/drm/drm_fourcc.h | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index e04613d30a13..9a97405a3d2a 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -298,6 +298,38 @@ extern "C" { */ #define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1)

+/*

• • Qualcomm Compressed Format
• • Refers to a compressed variant of the base format that is compressed.
• • Implementation may be platform and base-format specific.
• */

+#define DRM_FORMAT_MOD_QCOM_COMPRESSED fourcc_mod_code(QCOM, 1)

hmm, somehow (I guess rebasing gone wrong?) we end up with DRM_FORMAT_MOD_QCOM_COMPRESSED.. I'll drop the 2nd hunk

BR, -R

+/*

• • QTI DX Format
• • Refers to a DX variant of the base format.
• • Implementation may be platform and base-format specific.
• */

+#define DRM_FORMAT_MOD_QCOM_DX fourcc_mod_code(QCOM, 0x2)

+/*

• • QTI Tight Format
• • Refers to a tightly packed variant of the base format.
• • Implementation may be platform and base-format specific.
• */

+#define DRM_FORMAT_MOD_QCOM_TIGHT fourcc_mod_code(QCOM, 0x4)

+/*

• • QTI Tile Format
• • Refers to a tile variant of the base format.
• • Implementation may be platform and base-format specific.
• */

+#define DRM_FORMAT_MOD_QCOM_TILE fourcc_mod_code(QCOM, 0x8)

/* Vivante framebuffer modifiers */

/* @@ -405,6 +437,19 @@ extern "C" { */ #define DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED fourcc_mod_code(BROADCOM, 1)

+/*

• • MSM compressed format
• • Refers to the compressed variant of a base format.
• • Implementation may be platform and base-format specific.
• • Each macrotile consists of m x n (mostly 4 x 4) tiles.
• • Pixel data pitch/stride is aligned with macrotile width.
• • Pixel data height is aligned with macrotile height.
• • Entire pixel data buffer is aligned with 4k(bytes).
• */

+#define DRM_FORMAT_MOD_QCOM_COMPRESSED fourcc_mod_code(QCOM, 1)

#if defined(__cplusplus) }

#endif

Sean Paul, Software Engineer, Google / Chromium OS

On Monday 09 July 2018 11:01 PM, Sean Paul wrote:

From: Chandan Uddaraju chandanu@codeaurora.org

For dual dsi mode, the horizontal timing needs to be divided by half since both the dsi controllers will be driving this panel. Adjust the pixel clock and DSI timing accordingly.

Reviewed-by: Archit Taneja architt@codeaurora.org

Changes in V2: --Removed Change-Id from the commit text tags.

Changes in V3: --Instead of adjusting the DRM mode structure, divide the clocks and horizontal timings in DSI host just before configuring the values.

Signed-off-by: Chandan Uddaraju chandanu@codeaurora.org Signed-off-by: Sean Paul seanpaul@chromium.org

---

drivers/gpu/drm/msm/dsi/dsi.h | 6 ++- drivers/gpu/drm/msm/dsi/dsi_host.c | 55 +++++++++++++++++++++------ drivers/gpu/drm/msm/dsi/dsi_manager.c | 7 ++-- 3 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 70d9a9a47acd..01c38f67d699 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -162,7 +162,8 @@ void msm_dsi_host_cmd_xfer_commit(struct mipi_dsi_host *host, int msm_dsi_host_enable(struct mipi_dsi_host *host); int msm_dsi_host_disable(struct mipi_dsi_host *host); int msm_dsi_host_power_on(struct mipi_dsi_host *host,

• 	struct msm_dsi_phy_shared_timings *phy_shared_timings);
  

• 	struct msm_dsi_phy_shared_timings *phy_shared_timings,
  

• 	bool is_dual_dsi);
  

  int msm_dsi_host_power_off(struct mipi_dsi_host *host); int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host, struct drm_display_mode *mode);

@@ -175,7 +176,8 @@ int msm_dsi_host_set_src_pll(struct mipi_dsi_host *host, struct msm_dsi_pll *src_pll); void msm_dsi_host_reset_phy(struct mipi_dsi_host *host); void msm_dsi_host_get_phy_clk_req(struct mipi_dsi_host *host,

• struct msm_dsi_phy_clk_request *clk_req);

• struct msm_dsi_phy_clk_request *clk_req,
• bool is_dual_dsi); void msm_dsi_host_destroy(struct mipi_dsi_host *host); int msm_dsi_host_modeset_init(struct mipi_dsi_host *host, struct drm_device *dev);

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 2f1a2780658a..671039b7b75b 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -118,6 +118,7 @@ struct msm_dsi_host { struct clk *byte_intf_clk;

u32 byte_clk_rate;

• u32 pixel_clk_rate; u32 esc_clk_rate;

  /* DSI v2 specific clocks */

@@ -511,7 +512,7 @@ static int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) goto error; }

• ret = clk_set_rate(msm_host->pixel_clk, msm_host->mode->clock * 1000);

• ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); if (ret) { pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); goto error;

@@ -592,7 +593,7 @@ static int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host) goto error; }

• ret = clk_set_rate(msm_host->pixel_clk, msm_host->mode->clock * 1000);

• ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate); if (ret) { pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret); goto error;

@@ -662,7 +663,7 @@ static void dsi_link_clk_disable(struct msm_dsi_host *msm_host) } }

-static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host) +static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host, bool is_dual_dsi) { struct drm_display_mode *mode = msm_host->mode; const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd; @@ -676,14 +677,28 @@ static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host) }

pclk_rate = mode->clock * 1000;

• /*

• * For dual DSI mode, the current DRM mode has
  

• * the complete width of the panel. Since, the complete
  

• * panel is driven by two DSI controllers, the
  

• * the clock rates have to be split between
  

• * the two dsi controllers. Adjust the byte and
  

• * pixel clock rates for each dsi host accordingly.
  

• */
  

• if (is_dual_dsi)

• pclk_rate /= 2;
  

• if (lanes > 0) { msm_host->byte_clk_rate = (pclk_rate * bpp) / (8 * lanes); } else { pr_err("%s: forcing mdss_dsi lanes to 1\n", __func__); msm_host->byte_clk_rate = (pclk_rate * bpp) / 8; }
• msm_host->pixel_clk_rate = pclk_rate;

• DBG("pclk=%d, bclk=%d", pclk_rate, msm_host->byte_clk_rate);

• DBG("pclk=%d, bclk=%d", msm_host->pixel_clk_rate,

• 		msm_host->byte_clk_rate);
  
  

  msm_host->esc_clk_rate = clk_get_rate(msm_host->esc_clk);

@@ -885,7 +900,7 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host, bool enable, dsi_write(msm_host, REG_DSI_CTRL, data); }

-static void dsi_timing_setup(struct msm_dsi_host *msm_host) +static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_dual_dsi) { struct drm_display_mode *mode = msm_host->mode; u32 hs_start = 0, vs_start = 0; /* take sync start as 0 */ @@ -897,10 +912,26 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host) u32 ha_end = ha_start + mode->hdisplay; u32 va_start = v_total - mode->vsync_start; u32 va_end = va_start + mode->vdisplay;

• u32 hdisplay = mode->hdisplay; u32 wc;

  DBG("");

• /*

• * For dual DSI mode, the current DRM mode has
  

• * the complete width of the panel. Since, the complete
  

• * panel is driven by two DSI controllers, the horizontal
  

• * timings have to be split between the two dsi controllers.
  

• * Adjust the DSI host timing values accordingly.
  

• */
  

• if (is_dual_dsi) {

• h_total /= 2;
  

• hs_end /= 2;
  

• ha_start /= 2;
  

• ha_end /= 2;
  

• hdisplay /= 2;
  

• }

• if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) { dsi_write(msm_host, REG_DSI_ACTIVE_H, DSI_ACTIVE_H_START(ha_start) |

@@ -921,7 +952,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host) DSI_ACTIVE_VSYNC_VPOS_END(vs_end)); } else { /* command mode */ /* image data and 1 byte write_memory_start cmd */

• wc = mode->hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
  

• wc = hdisplay * dsi_get_bpp(msm_host->format) / 8 + 1;
  
  

  dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM_CTRL, DSI_CMD_MDP_STREAM_CTRL_WORD_COUNT(wc) |

@@ -931,7 +962,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host) MIPI_DSI_DCS_LONG_WRITE));

dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM_TOTAL,

• 	DSI_CMD_MDP_STREAM_TOTAL_H_TOTAL(mode->hdisplay) |
  

• 	DSI_CMD_MDP_STREAM_TOTAL_H_TOTAL(hdisplay) |
  DSI_CMD_MDP_STREAM_TOTAL_V_TOTAL(mode->vdisplay));
  

  } }

@@ -2189,12 +2220,13 @@ void msm_dsi_host_reset_phy(struct mipi_dsi_host *host) }

void msm_dsi_host_get_phy_clk_req(struct mipi_dsi_host *host,

• struct msm_dsi_phy_clk_request *clk_req)

• 	struct msm_dsi_phy_clk_request *clk_req,
  

• 	bool is_dual_dsi)
  

  { struct msm_dsi_host *msm_host = to_msm_dsi_host(host); int ret;

• ret = dsi_calc_clk_rate(msm_host);

• ret = dsi_calc_clk_rate(msm_host, is_dual_dsi); if (ret) { pr_err("%s: unable to calc clk rate, %d\n", __func__, ret); return;

@@ -2256,7 +2288,8 @@ static void msm_dsi_sfpb_config(struct msm_dsi_host *msm_host, bool enable) }

int msm_dsi_host_power_on(struct mipi_dsi_host *host,

• 	struct msm_dsi_phy_shared_timings *phy_shared_timings)
  

• 	struct msm_dsi_phy_shared_timings *phy_shared_timings,
  

• 	bool is_dual_dsi)
  

  { struct msm_dsi_host *msm_host = to_msm_dsi_host(host); int ret = 0;

@@ -2291,7 +2324,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host, goto fail_disable_clk; }

• dsi_timing_setup(msm_host);

• dsi_timing_setup(msm_host, is_dual_dsi); dsi_sw_reset(msm_host); dsi_ctrl_config(msm_host, true, phy_shared_timings);

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index 4cb1cb68878b..3bb506b44a4b 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -134,8 +134,9 @@ static int enable_phy(struct msm_dsi *msm_dsi, int src_pll_id, { struct msm_dsi_phy_clk_request clk_req; int ret;

• bool is_dual_dsi = IS_DUAL_DSI();

• msm_dsi_host_get_phy_clk_req(msm_dsi->host, &clk_req);

• msm_dsi_host_get_phy_clk_req(msm_dsi->host, &clk_req, is_dual_dsi);

  ret = msm_dsi_phy_enable(msm_dsi->phy, src_pll_id, &clk_req); msm_dsi_phy_get_shared_timings(msm_dsi->phy, shared_timings);

@@ -458,7 +459,7 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge) if (is_dual_dsi && (DSI_1 == id)) return;

• ret = msm_dsi_host_power_on(host, &phy_shared_timings[id]);

• ret = msm_dsi_host_power_on(host, &phy_shared_timings[id], is_dual_dsi); if (ret) { pr_err("%s: power on host %d failed, %d\n", __func__, id, ret); goto host_on_fail;

@@ -466,7 +467,7 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge *bridge)

if (is_dual_dsi && msm_dsi1) { ret = msm_dsi_host_power_on(msm_dsi1->host,

• 			    &phy_shared_timings[DSI_1]);
  

• 		&phy_shared_timings[DSI_1], is_dual_dsi);
  

  if (ret) { pr_err("%s: power on host1 failed, %d\n", __func__, ret);

On Monday 09 July 2018 11:01 PM, Sean Paul wrote:

From: Rajesh Yadav ryadav@codeaurora.org

SoCs having mdp5 or dpu have identical tree like device hierarchy where MDSS top level wrapper manages common power resources for all child devices.

Subclass msm_mdss so that msm_mdss includes common defines and mdp5/dpu mdss derivations to include any extensions.

Add mdss helper interface (msm_mdss_funcs) to msm_mdss base for mdp5/dpu mdss specific implementation calls.

This change subclasses msm_mdss for mdp5, dpu specific changes will be done separately.

Reviewed-by: Archit Taneja architt@codeaurora.org

Changes in v3:

• none

Changes in v2:

• fixed indentation for irq_domain_add_linear call (Sean Paul)

Signed-off-by: Rajesh Yadav ryadav@codeaurora.org Reviewed-by: Sean Paul seanpaul@chromium.org [seanpaul rebased on msm-next and resolved conflicts] Signed-off-by: Sean Paul seanpaul@chromium.org

---

drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c | 154 ++++++++++++---------- drivers/gpu/drm/msm/msm_drv.c | 22 +++- drivers/gpu/drm/msm/msm_kms.h | 17 ++- 3 files changed, 109 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c index f2a0db7a8a03..1cc4e57f0226 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mdss.c @@ -20,12 +20,10 @@ #include "msm_drv.h" #include "mdp5_kms.h"

-/*

• • If needed, this can become more specific: something like struct mdp5_mdss,
• • which contains a 'struct msm_mdss base' member.
• */

-struct msm_mdss {

• struct drm_device *dev;

+#define to_mdp5_mdss(x) container_of(x, struct mdp5_mdss, base)

+struct mdp5_mdss {

• struct msm_mdss base;

  void __iomem *mmio, *vbif;

@@ -41,22 +39,22 @@ struct msm_mdss { } irqcontroller; };

-static inline void mdss_write(struct msm_mdss *mdss, u32 reg, u32 data) +static inline void mdss_write(struct mdp5_mdss *mdp5_mdss, u32 reg, u32 data) {

• msm_writel(data, mdss->mmio + reg);

• msm_writel(data, mdp5_mdss->mmio + reg); }

-static inline u32 mdss_read(struct msm_mdss *mdss, u32 reg) +static inline u32 mdss_read(struct mdp5_mdss *mdp5_mdss, u32 reg) {

• return msm_readl(mdss->mmio + reg);

• return msm_readl(mdp5_mdss->mmio + reg); }

  static irqreturn_t mdss_irq(int irq, void *arg) {

• struct msm_mdss *mdss = arg;

• struct mdp5_mdss *mdp5_mdss = arg; u32 intr;

• intr = mdss_read(mdss, REG_MDSS_HW_INTR_STATUS);

• intr = mdss_read(mdp5_mdss, REG_MDSS_HW_INTR_STATUS);

  VERB("intr=%08x", intr);

@@ -64,7 +62,7 @@ static irqreturn_t mdss_irq(int irq, void *arg) irq_hw_number_t hwirq = fls(intr) - 1;

generic_handle_irq(irq_find_mapping(

• 		mdss->irqcontroller.domain, hwirq));
  

• 		mdp5_mdss->irqcontroller.domain, hwirq));
  

  intr &= ~(1 << hwirq); }

@@ -84,19 +82,19 @@ static irqreturn_t mdss_irq(int irq, void *arg)

static void mdss_hw_mask_irq(struct irq_data *irqd) {

• struct msm_mdss *mdss = irq_data_get_irq_chip_data(irqd);

• struct mdp5_mdss *mdp5_mdss = irq_data_get_irq_chip_data(irqd);

  smp_mb__before_atomic();

• clear_bit(irqd->hwirq, &mdss->irqcontroller.enabled_mask);

• clear_bit(irqd->hwirq, &mdp5_mdss->irqcontroller.enabled_mask); smp_mb__after_atomic(); }

  static void mdss_hw_unmask_irq(struct irq_data *irqd) {

• struct msm_mdss *mdss = irq_data_get_irq_chip_data(irqd);

• struct mdp5_mdss *mdp5_mdss = irq_data_get_irq_chip_data(irqd);

  smp_mb__before_atomic();

• set_bit(irqd->hwirq, &mdss->irqcontroller.enabled_mask);

• set_bit(irqd->hwirq, &mdp5_mdss->irqcontroller.enabled_mask); smp_mb__after_atomic(); }

@@ -109,13 +107,13 @@ static struct irq_chip mdss_hw_irq_chip = { static int mdss_hw_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq) {

• struct msm_mdss *mdss = d->host_data;

• struct mdp5_mdss *mdp5_mdss = d->host_data;

  if (!(VALID_IRQS & (1 << hwirq))) return -EPERM;

  irq_set_chip_and_handler(irq, &mdss_hw_irq_chip, handle_level_irq);

• irq_set_chip_data(irq, mdss);

• irq_set_chip_data(irq, mdp5_mdss);

  return 0; }

@@ -126,90 +124,99 @@ static const struct irq_domain_ops mdss_hw_irqdomain_ops = { };

-static int mdss_irq_domain_init(struct msm_mdss *mdss) +static int mdss_irq_domain_init(struct mdp5_mdss *mdp5_mdss) {

• struct device *dev = mdss->dev->dev;

• struct device *dev = mdp5_mdss->base.dev->dev; struct irq_domain *d;

  d = irq_domain_add_linear(dev->of_node, 32, &mdss_hw_irqdomain_ops,

• 		  mdss);
  

• 		  mdp5_mdss);
  

  if (!d) { dev_err(dev, "mdss irq domain add failed\n"); return -ENXIO; }

• mdss->irqcontroller.enabled_mask = 0;
• mdss->irqcontroller.domain = d;

• mdp5_mdss->irqcontroller.enabled_mask = 0;

• mdp5_mdss->irqcontroller.domain = d;

  return 0; }

-int msm_mdss_enable(struct msm_mdss *mdss) +static int mdp5_mdss_enable(struct msm_mdss *mdss) {

• struct mdp5_mdss *mdp5_mdss = to_mdp5_mdss(mdss); DBG("");

• clk_prepare_enable(mdss->ahb_clk);
• if (mdss->axi_clk)

• clk_prepare_enable(mdss->axi_clk);
  

• if (mdss->vsync_clk)

• clk_prepare_enable(mdss->vsync_clk);
  

• clk_prepare_enable(mdp5_mdss->ahb_clk);

• if (mdp5_mdss->axi_clk)

• clk_prepare_enable(mdp5_mdss->axi_clk);
  

• if (mdp5_mdss->vsync_clk)

• clk_prepare_enable(mdp5_mdss->vsync_clk);
  
  

  return 0; }

-int msm_mdss_disable(struct msm_mdss *mdss) +static int mdp5_mdss_disable(struct msm_mdss *mdss) {

• struct mdp5_mdss *mdp5_mdss = to_mdp5_mdss(mdss); DBG("");

• if (mdss->vsync_clk)

• clk_disable_unprepare(mdss->vsync_clk);
  

• if (mdss->axi_clk)

• clk_disable_unprepare(mdss->axi_clk);
  

• clk_disable_unprepare(mdss->ahb_clk);

• if (mdp5_mdss->vsync_clk)

• clk_disable_unprepare(mdp5_mdss->vsync_clk);
  

• if (mdp5_mdss->axi_clk)

• clk_disable_unprepare(mdp5_mdss->axi_clk);
  

• clk_disable_unprepare(mdp5_mdss->ahb_clk);

  return 0; }

-static int msm_mdss_get_clocks(struct msm_mdss *mdss) +static int msm_mdss_get_clocks(struct mdp5_mdss *mdp5_mdss) {

• struct platform_device *pdev = to_platform_device(mdss->dev->dev);

• struct platform_device *pdev =

• 	to_platform_device(mdp5_mdss->base.dev->dev);
  
  

• mdss->ahb_clk = msm_clk_get(pdev, "iface");
• if (IS_ERR(mdss->ahb_clk))

• mdss->ahb_clk = NULL;
  

• mdp5_mdss->ahb_clk = msm_clk_get(pdev, "iface");
• if (IS_ERR(mdp5_mdss->ahb_clk))

• mdp5_mdss->ahb_clk = NULL;
  
  

• mdss->axi_clk = msm_clk_get(pdev, "bus");
• if (IS_ERR(mdss->axi_clk))

• mdss->axi_clk = NULL;
  

• mdp5_mdss->axi_clk = msm_clk_get(pdev, "bus");
• if (IS_ERR(mdp5_mdss->axi_clk))

• mdp5_mdss->axi_clk = NULL;
  
  

• mdss->vsync_clk = msm_clk_get(pdev, "vsync");
• if (IS_ERR(mdss->vsync_clk))

• mdss->vsync_clk = NULL;
  

• mdp5_mdss->vsync_clk = msm_clk_get(pdev, "vsync");

• if (IS_ERR(mdp5_mdss->vsync_clk))

• mdp5_mdss->vsync_clk = NULL;
  
  

  return 0; }

-void msm_mdss_destroy(struct drm_device *dev) +static void mdp5_mdss_destroy(struct drm_device *dev) { struct msm_drm_private *priv = dev->dev_private;

• struct msm_mdss *mdss = priv->mdss;

• struct mdp5_mdss *mdp5_mdss = to_mdp5_mdss(priv->mdss);

• if (!mdss)

• if (!mdp5_mdss) return;

• irq_domain_remove(mdss->irqcontroller.domain);
• mdss->irqcontroller.domain = NULL;

• irq_domain_remove(mdp5_mdss->irqcontroller.domain);
• mdp5_mdss->irqcontroller.domain = NULL;

• regulator_disable(mdss->vdd);

• regulator_disable(mdp5_mdss->vdd);

  pm_runtime_disable(dev->dev); }

-int msm_mdss_init(struct drm_device *dev) +static const struct msm_mdss_funcs mdss_funcs = {

• .enable = mdp5_mdss_enable,
• .disable = mdp5_mdss_disable,
• .destroy = mdp5_mdss_destroy,

+};

+int mdp5_mdss_init(struct drm_device *dev) { struct platform_device *pdev = to_platform_device(dev->dev); struct msm_drm_private *priv = dev->dev_private;

• struct msm_mdss *mdss;

• struct mdp5_mdss *mdp5_mdss; int ret;

  DBG("");

@@ -217,40 +224,40 @@ int msm_mdss_init(struct drm_device *dev) if (!of_device_is_compatible(dev->dev->of_node, "qcom,mdss")) return 0;

• mdss = devm_kzalloc(dev->dev, sizeof(*mdss), GFP_KERNEL);
• if (!mdss) {

• mdp5_mdss = devm_kzalloc(dev->dev, sizeof(*mdp5_mdss), GFP_KERNEL);
• if (!mdp5_mdss) { ret = -ENOMEM; goto fail; }

• mdss->dev = dev;

• mdp5_mdss->base.dev = dev;

• mdss->mmio = msm_ioremap(pdev, "mdss_phys", "MDSS");
• if (IS_ERR(mdss->mmio)) {

• ret = PTR_ERR(mdss->mmio);
  

• mdp5_mdss->mmio = msm_ioremap(pdev, "mdss_phys", "MDSS");
• if (IS_ERR(mdp5_mdss->mmio)) {

• ret = PTR_ERR(mdp5_mdss->mmio);
  

  goto fail; }

• mdss->vbif = msm_ioremap(pdev, "vbif_phys", "VBIF");
• if (IS_ERR(mdss->vbif)) {

• ret = PTR_ERR(mdss->vbif);
  

• mdp5_mdss->vbif = msm_ioremap(pdev, "vbif_phys", "VBIF");
• if (IS_ERR(mdp5_mdss->vbif)) {

• ret = PTR_ERR(mdp5_mdss->vbif);
  

  goto fail; }

• ret = msm_mdss_get_clocks(mdss);

• ret = msm_mdss_get_clocks(mdp5_mdss); if (ret) { dev_err(dev->dev, "failed to get clocks: %d\n", ret); goto fail; }

  /* Regulator to enable GDSCs in downstream kernels */

• mdss->vdd = devm_regulator_get(dev->dev, "vdd");
• if (IS_ERR(mdss->vdd)) {

• ret = PTR_ERR(mdss->vdd);
  

• mdp5_mdss->vdd = devm_regulator_get(dev->dev, "vdd");
• if (IS_ERR(mdp5_mdss->vdd)) {

• ret = PTR_ERR(mdp5_mdss->vdd);
  

  goto fail; }

• ret = regulator_enable(mdss->vdd);

• ret = regulator_enable(mdp5_mdss->vdd); if (ret) { dev_err(dev->dev, "failed to enable regulator vdd: %d\n", ret);

@@ -258,25 +265,26 @@ int msm_mdss_init(struct drm_device *dev) }

ret = devm_request_irq(dev->dev, platform_get_irq(pdev, 0),

• 	       mdss_irq, 0, "mdss_isr", mdss);
  

• 	       mdss_irq, 0, "mdss_isr", mdp5_mdss);
  

  if (ret) { dev_err(dev->dev, "failed to init irq: %d\n", ret); goto fail_irq; }

• ret = mdss_irq_domain_init(mdss);

• ret = mdss_irq_domain_init(mdp5_mdss); if (ret) { dev_err(dev->dev, "failed to init sub-block irqs: %d\n", ret); goto fail_irq; }

• priv->mdss = mdss;

• mdp5_mdss->base.funcs = &mdss_funcs;

• priv->mdss = &mdp5_mdss->base;

  pm_runtime_enable(dev->dev);

  return 0; fail_irq:

• regulator_disable(mdss->vdd);

• regulator_disable(mdp5_mdss->vdd); fail: return ret; }

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 7f7321eb5312..2608d3f77956 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -208,6 +208,7 @@ static int msm_drm_uninit(struct device *dev) struct drm_device *ddev = platform_get_drvdata(pdev); struct msm_drm_private *priv = ddev->dev_private; struct msm_kms *kms = priv->kms;

• struct msm_mdss *mdss = priv->mdss; struct msm_vblank_ctrl *vbl_ctrl = &priv->vblank_ctrl; struct vblank_event *vbl_ev, *tmp;

@@ -258,7 +259,8 @@ static int msm_drm_uninit(struct device *dev)

component_unbind_all(dev, ddev);

• msm_mdss_destroy(ddev);

• if (mdss && mdss->funcs)

• mdss->funcs->destroy(ddev);
  
  

  ddev->dev_private = NULL; drm_dev_unref(ddev);

@@ -357,6 +359,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) struct drm_device *ddev; struct msm_drm_private *priv; struct msm_kms *kms;

• struct msm_mdss *mdss; int ret;

  ddev = drm_dev_alloc(drv, dev);

@@ -376,13 +379,15 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) ddev->dev_private = priv; priv->dev = ddev;

• ret = msm_mdss_init(ddev);

• ret = mdp5_mdss_init(ddev); if (ret) { kfree(priv); drm_dev_unref(ddev); return ret; }

• mdss = priv->mdss;

• priv->wq = alloc_ordered_workqueue("msm", 0); priv->atomic_wq = alloc_ordered_workqueue("msm:atomic", 0);

@@ -396,7 +401,8 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) /* Bind all our sub-components: */ ret = component_bind_all(dev, ddev); if (ret) {

• msm_mdss_destroy(ddev);
  

• if (mdss && mdss->funcs)
  

• 	mdss->funcs->destroy(ddev);
  

  kfree(priv); drm_dev_unref(ddev); return ret;

@@ -924,11 +930,12 @@ static int msm_runtime_suspend(struct device *dev) { struct drm_device *ddev = dev_get_drvdata(dev); struct msm_drm_private *priv = ddev->dev_private;

• struct msm_mdss *mdss = priv->mdss;

  DBG("");

• if (priv->mdss)

• return msm_mdss_disable(priv->mdss);
  

• if (mdss && mdss->funcs)

• return mdss->funcs->disable(mdss);
  
  

  return 0; }

@@ -937,11 +944,12 @@ static int msm_runtime_resume(struct device *dev) { struct drm_device *ddev = dev_get_drvdata(dev); struct msm_drm_private *priv = ddev->dev_private;

• struct msm_mdss *mdss = priv->mdss;

  DBG("");

• if (priv->mdss)

• return msm_mdss_enable(priv->mdss);
  

• if (mdss && mdss->funcs)

• return mdss->funcs->enable(mdss);
  
  

  return 0; }

diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h index dfd92947de2c..76c14221ffdf 100644 --- a/drivers/gpu/drm/msm/msm_kms.h +++ b/drivers/gpu/drm/msm/msm_kms.h @@ -86,9 +86,18 @@ static inline void msm_kms_init(struct msm_kms *kms,

struct msm_kms *mdp4_kms_init(struct drm_device *dev); struct msm_kms *mdp5_kms_init(struct drm_device *dev); -int msm_mdss_init(struct drm_device *dev); -void msm_mdss_destroy(struct drm_device *dev); -int msm_mdss_enable(struct msm_mdss *mdss); -int msm_mdss_disable(struct msm_mdss *mdss);

+struct msm_mdss_funcs {

• int (*enable)(struct msm_mdss *mdss);
• int (*disable)(struct msm_mdss *mdss);
• void (*destroy)(struct drm_device *dev);

+};

+struct msm_mdss {

• struct drm_device *dev;
• const struct msm_mdss_funcs *funcs;

+};

+int mdp5_mdss_init(struct drm_device *dev);

#endif /* __MSM_KMS_H__ */

From: Jeykumar Sankaran jsanka@codeaurora.org

Adds bindings for Snapdragon 845 display processing unit

Changes in v2: - Use SoC specific compatibles for mdss and dpu - Use assigned-clocks to set initial clock frequency

Signed-off-by: Jeykumar Sankaran jsanka@codeaurora.org Signed-off-by: Rajesh Yadav ryadav@codeaurora.org Signed-off-by: Sean Paul seanpaul@chromium.org --- .../devicetree/bindings/display/msm/dpu.txt | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/msm/dpu.txt

diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt new file mode 100644 index 000000000000..a998028896ba --- /dev/null +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -0,0 +1,136 @@ +Qualcomm Technologies, Inc. DPU KMS + +Description: + +Device tree bindings for MSM Mobile Display Subsytem(MDSS) that encapsulates +sub-blocks like DPU display controller, DSI and DP interfaces etc. +The DPU display controller is found in SDM845 SoC. + +MDSS: +Required properties: +- compatible: "qcom,sdm845-mdss" +- reg: physical base address and length of contoller's registers. +- reg-names: register region names. The following region is required: + * "mdss_phys" +- power-domains: a power domain consumer specifier according to + Documentation/devicetree/bindings/power/power_domain.txt +- clocks: list of phandles for clock device nodes needed by the device. +- clock-names: device clock names, must be in same order as clocks property. + The following clocks are required: + * "iface" + * "bus" + * "core" +- interrupts: interrupt signal from MDSS. +- interrupt-controller: identifies the node as an interrupt controller. +- #interrupt-cells: specifies the number of cells needed to encode an interrupt + source, should be 1. +- iommus: phandle of iommu device node. +- #address-cells: number of address cells for the MDSS children. Should be 1. +- #size-cells: Should be 1. +- ranges: parent bus address space is the same as the child bus address space. + +Optional properties: +- assigned-clocks: list of phandles for clock device nodes needing rate + assignment +- assigned-clock-rates: list of clock frequencies sorted in the same order as + the assigned-clocks property. + +MDP: +Required properties: +- compatible: "qcom,sdm845-dpu" +- reg: physical base address and length of controller's registers. +- reg-names : register region names. The following region is required: + * "mdp_phys" + * "vbif_phys" +- clocks: list of phandles for clock device nodes needed by the device. +- clock-names: device clock names, must be in same order as clocks property. + The following clocks are required. + * "bus" + * "iface" + * "core" + * "vsync" +- interrupt-parent: phandle to MDSS block. +- interrupts: interrupt line from DPU to MDSS. +- ports: contains the list of output ports from DPU device. These ports connect + to interfaces that are external to the DPU hardware, such as DSI, DP etc. + + Each output port contains an endpoint that describes how it is connected to an + external interface. These are described by the standard properties documented + here: + Documentation/devicetree/bindings/graph.txt + Documentation/devicetree/bindings/media/video-interfaces.txt + + Port 0 -> DPU_INTF1 (DSI1) + Port 1 -> DPU_INTF2 (DSI2) + +Optional properties: +- assigned-clocks: list of phandles for clock device nodes needing rate + assignment +- assigned-clock-rates: list of clock frequencies sorted in the same order as + the assigned-clocks property. + +Example: + + mdss: mdss@ae00000 { + compatible = "qcom,sdm845-mdss"; + reg = <0xae00000 0x1000>; + reg-names = "mdss_phys"; + + power-domains = <&clock_dispcc 0>; + + clocks = <&gcc GCC_DISP_AHB_CLK>, <&gcc GCC_DISP_AXI_CLK>, + <&clock_dispcc DISP_CC_MDSS_MDP_CLK>; + clock-names = "iface", "bus", "core"; + + assigned-clocks = <&clock_dispcc DISP_CC_MDSS_MDP_CLK>; + assigned-clock-rates = <300000000>; + + interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>; + interrupt-controller; + #interrupt-cells = <1>; + + iommus = <&apps_iommu 0>; + + #address-cells = <1>; + #size-cells = <1>; + ranges; + + mdss_mdp: mdp@ae01000 { + compatible = "qcom,sdm845-dpu"; + reg = <0x0ae01000 0x8f000>, + <0x0aeb0000 0x2008>; + reg-names = "mdp_phys", "vbif_phys"; + + clocks = <&clock_dispcc DISP_CC_MDSS_AHB_CLK>, + <&clock_dispcc DISP_CC_MDSS_AXI_CLK>, + <&clock_dispcc DISP_CC_MDSS_MDP_CLK>, + <&clock_dispcc DISP_CC_MDSS_VSYNC_CLK>; + clock-names = "iface", "bus", "core", "vsync"; + + assigned-clocks = <&clock_dispcc DISP_CC_MDSS_MDP_CLK>, + <&clock_dispcc DISP_CC_MDSS_VSYNC_CLK>; + assigned-clock-rates = <0 0 300000000 19200000>; + + interrupt-parent = <&mdss>; + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dpu_intf1_out: endpoint { + remote-endpoint = <&dsi0_in>; + }; + }; + + port@1 { + reg = <1>; + dpu_intf2_out: endpoint { + remote-endpoint = <&dsi1_in>; + }; + }; + }; + }; + };

On Mon, Jul 09, 2018 at 12:07:11PM -0600, Rob Herring wrote:

On Mon, Jul 9, 2018 at 11:40 AM Sean Paul seanpaul@chromium.org wrote:

...

Signed-off-by: Sean Paul seanpaul@chromium.org

arch/arm64/boot/dts/qcom/sdm845.dtsi | 194 +++++++++++++++++++++++++++ 1 file changed, 194 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index cdaabeb3c995..339afed856de 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -5,6 +5,8 @@

• Copyright (c) 2018, The Linux Foundation. All rights reserved.

*/

+#include <dt-bindings/clock/qcom,dispcc-sdm845.h> +#include <dt-bindings/clock/qcom,gcc-sdm845.h> #include <dt-bindings/interrupt-controller/arm-gic.h>

/ { @@ -221,6 +223,198 @@ #interrupt-cells = <2>; };

•           mdss: mdss@ae00000 {
  

•                   compatible = "qcom,dpu-mdss";
  

•                   reg = <0xae00000 0x1000>;
  

•                   reg-names = "mdss_phys";
  

•                   power-domains = <&dispcc 0>;
  

•                   clocks = <&gcc GCC_DISP_AHB_CLK>,
  

•                            <&gcc GCC_DISP_AXI_CLK>,
  

•                            <&dispcc DISP_CC_MDSS_MDP_CLK>;
  

•                   clock-names = "iface", "bus", "core";
  

•                   clock-frequency = <0 0 300000000>;
  

•                   interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
  

•                   interrupt-controller;
  

•                   #interrupt-cells = <1>;
  

•                   /* iommus = <&apps_iommu 0>; */
  

•                   #address-cells = <1>;
  

•                   #size-cells = <1>;
  

•                   ranges;
  

•                   mdss_mdp: mdp@ae01000 {
  

•                           compatible = "qcom,dpu";
  

Hi Rob, Thanks for the quick turnaround! In addition to below, I'll also beef up the commit message, since I forgot to add any description of the change.

Needs an SoC specific compatible. Did this binding get reviewed?

No, it's part of this set ([PATCH 19/21] dt-bindings: msm/disp: Add bindings for Snapdragon 845 DPU).

...

•                           reg = <0x0ae01000 0x8f000>,
  

•                                 <0x0aeb0000 0x2008>;
  

•                           reg-names = "mdp_phys", "vbif_phys";
  

•                           clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_AXI_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_MDP_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
  

•                           clock-names = "iface", "bus", "core", "vsync";
  

•                           clock-frequency = <0 0 300000000 19200000>;
  

That's abusing clock-frequency which is generally 1 value. Use assigned-clock-rates instead.

Thanks, will change.

...

•                           interrupt-parent = <&mdss>;
  

•                           interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
  

•                           ports {
  

•                                   #address-cells = <1>;
  

•                                   #size-cells = <0>;
  

•                                   port@0 {
  

•                                           reg = <0>;
  

•                                           dpu_intf1_out: endpoint {
  

•                                                   remote-endpoint = <&dsi0_in>;
  

•                                           };
  

•                                   };
  

•                                   port@1 {
  

•                                           reg = <1>;
  

•                                           dpu_intf2_out: endpoint {
  

•                                                   remote-endpoint = <&dsi1_in>;
  

•                                           };
  

•                                   };
  

•                           };
  

•                   };
  

•                   dsi0: dsi@ae94000 {
  

•                           compatible = "qcom,mdss-dsi-ctrl";
  

Needs an SoC specific compatible.

Ok, will add.

...

•                           reg = <0xae94000 0x400>;
  

•                           reg-names = "dsi_ctrl";
  

•                           interrupt-parent = <&mdss>;
  

•                           interrupts = <4 0>;
  

•                           clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_PCLK0_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_ESC0_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_AHB_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_AXI_CLK>;
  

•                           clock-names = "byte_clk",
  

•                                         "byte_intf_clk",
  

•                                         "pixel_clk",
  

•                                         "core_clk",
  

•                                         "iface_clk",
  

•                                         "bus_clk";
  

Should have found this in binding review, but the "_clk" part is redundant.

This binding is already in mainline (Documentation/devicetree/bindings/display/msm/dsi.txt), which is why _clk is here, but not above.

Sean

...

•                           phys = <&dsi0_phy>;
  

•                           phy-names = "dsi-phy";
  

•                           #address-cells = <1>;
  

•                           #size-cells = <0>;
  

•                           ports {
  

•                                   #address-cells = <1>;
  

•                                   #size-cells = <0>;
  

•                                   port@0 {
  

•                                           reg = <0>;
  

•                                           dsi0_in: endpoint {
  

•                                                   remote-endpoint = <&dpu_intf1_out>;
  

•                                           };
  

•                                   };
  

•                                   port@1 {
  

•                                           reg = <1>;
  

•                                           dsi0_out: endpoint {
  

•                                           };
  

•                                   };
  

•                           };
  

•                   };
  

•                   dsi0_phy: dsi-phy@ae94400 {
  

•                           compatible = "qcom,dsi-phy-10nm";
  

•                           reg = <0xae94400 0x200>,
  

•                                 <0xae94a00 0x1e0>,
  

•                                 <0xae94600 0x280>;
  

•                           reg-names = "dsi_phy",
  

•                                       "dsi_pll",
  

•                                       "dsi_phy_lane";
  

•                           #clock-cells = <1>;
  

•                           #phy-cells = <0>;
  

•                           clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
  

•                           clock-names = "iface_clk";
  

•                   };
  

•                   dsi1: dsi@ae96000 {
  

•                           compatible = "qcom,mdss-dsi-ctrl";
  

•                           reg = <0xae96000 0x400>;
  

•                           reg-names = "dsi_ctrl";
  

•                           interrupt-parent = <&mdss>;
  

•                           interrupts = <5 0>;
  

•                           clocks = <&dispcc DISP_CC_MDSS_BYTE1_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_BYTE1_INTF_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_PCLK1_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_ESC1_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_AHB_CLK>,
  

•                                    <&dispcc DISP_CC_MDSS_AXI_CLK>;
  

•                           clock-names = "byte_clk",
  

•                                         "byte_intf_clk",
  

•                                         "pixel_clk",
  

•                                         "core_clk",
  

•                                         "iface_clk",
  

•                                         "bus_clk";
  

•                           phys = <&dsi1_phy>;
  

•                           phy-names = "dsi-phy";
  

•                           #address-cells = <1>;
  

•                           #size-cells = <0>;
  

•                           ports {
  

•                                   #address-cells = <1>;
  

•                                   #size-cells = <0>;
  

•                                   port@0 {
  

•                                           reg = <0>;
  

•                                           dsi1_in: endpoint {
  

•                                                   remote-endpoint = <&dpu_intf2_out>;
  

•                                           };
  

•                                   };
  

•                                   port@1 {
  

•                                           reg = <1>;
  

•                                           dsi1_out: endpoint {
  

•                                           };
  

•                                   };
  

•                           };
  

•                   };
  

•                   dsi1_phy: dsi-phy@ae96400 {
  

•                           compatible = "qcom,dsi-phy-10nm";
  

•                           reg = <0xae96400 0x200>,
  

•                                 <0xae96a00 0x10e>,
  

•                                 <0xae96600 0x280>;
  

•                           reg-names = "dsi_phy",
  

•                                       "dsi_pll",
  

•                                       "dsi_phy_lane";
  

•                           #clock-cells = <1>;
  

•                           #phy-cells = <0>;
  

•                           clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
  

•                           clock-names = "iface_clk";
  

•                   };
  

•           };
  

•           dispcc: qcom,dispcc@af00000 {
  

•                   compatible = "qcom,sdm845-dispcc";
  

•                   reg = <0xaf00000 0x10000>;
  

•                   #clock-cells = <1>;
  

•                   #reset-cells = <1>;
  

•                   #power-domain-cells = <1>;
  

•           };
  

•           spmi_bus: spmi@c440000 {
                    compatible = "qcom,spmi-pmic-arb";
                    reg = <0xc440000 0x1100>,
  

-- Sean Paul, Software Engineer, Google / Chromium OS

DPU is short for the Display Processing Unit. It is the display controller on Qualcomm SDM845 chips.

While the dts is pretty sparse for sdm845 atm, the only piece we're missing is the iommu. It's commented out for now, and should be uncommented once support is provided.

Changes in v2: - Beefed up commit message - Use SoC specific compatibles for mdss and dpu - Use assigned-clocks to set initial clock frequency

Signed-off-by: Sean Paul seanpaul@chromium.org --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 200 +++++++++++++++++++++++++++ 1 file changed, 200 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index cdaabeb3c995..f1f4cdc9cb63 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -5,6 +5,8 @@ * Copyright (c) 2018, The Linux Foundation. All rights reserved. */

+#include <dt-bindings/clock/qcom,dispcc-sdm845.h> +#include <dt-bindings/clock/qcom,gcc-sdm845.h> #include <dt-bindings/interrupt-controller/arm-gic.h>

/ { @@ -221,6 +223,204 @@ #interrupt-cells = <2>; };

+ mdss: mdss@ae00000 { + compatible = "qcom,sdm845-mdss"; + reg = <0xae00000 0x1000>; + reg-names = "mdss_phys"; + + power-domains = <&dispcc 0>; + + clocks = <&gcc GCC_DISP_AHB_CLK>, + <&gcc GCC_DISP_AXI_CLK>, + <&dispcc DISP_CC_MDSS_MDP_CLK>; + clock-names = "iface", "bus", "core"; + + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>; + assigned-clock-rates = <300000000>; + + interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>; + interrupt-controller; + #interrupt-cells = <1>; + + /* iommus = <&apps_iommu 0>; */ + + #address-cells = <1>; + #size-cells = <1>; + ranges; + + mdss_mdp: mdp@ae01000 { + compatible = "qcom,sdm845-dpu"; + reg = <0x0ae01000 0x8f000>, + <0x0aeb0000 0x2008>; + reg-names = "mdp_phys", "vbif_phys"; + + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&dispcc DISP_CC_MDSS_AXI_CLK>, + <&dispcc DISP_CC_MDSS_MDP_CLK>, + <&dispcc DISP_CC_MDSS_VSYNC_CLK>; + clock-names = "iface", "bus", "core", "vsync"; + + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>, + <&dispcc DISP_CC_MDSS_VSYNC_CLK>; + assigned-clock-rates = <300000000>, + <19200000>; + + interrupt-parent = <&mdss>; + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dpu_intf1_out: endpoint { + remote-endpoint = <&dsi0_in>; + }; + }; + + port@1 { + reg = <1>; + dpu_intf2_out: endpoint { + remote-endpoint = <&dsi1_in>; + }; + }; + }; + }; + + dsi0: dsi@ae94000 { + compatible = "qcom,mdss-dsi-ctrl"; + reg = <0xae94000 0x400>; + reg-names = "dsi_ctrl"; + + interrupt-parent = <&mdss>; + interrupts = <4 0>; + + clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>, + <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>, + <&dispcc DISP_CC_MDSS_PCLK0_CLK>, + <&dispcc DISP_CC_MDSS_ESC0_CLK>, + <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&dispcc DISP_CC_MDSS_AXI_CLK>; + clock-names = "byte_clk", + "byte_intf_clk", + "pixel_clk", + "core_clk", + "iface_clk", + "bus_clk"; + + phys = <&dsi0_phy>; + phy-names = "dsi-phy"; + + #address-cells = <1>; + #size-cells = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dsi0_in: endpoint { + remote-endpoint = <&dpu_intf1_out>; + }; + }; + + port@1 { + reg = <1>; + dsi0_out: endpoint { + }; + }; + }; + }; + + dsi0_phy: dsi-phy@ae94400 { + compatible = "qcom,dsi-phy-10nm"; + reg = <0xae94400 0x200>, + <0xae94a00 0x1e0>, + <0xae94600 0x280>; + reg-names = "dsi_phy", + "dsi_pll", + "dsi_phy_lane"; + + #clock-cells = <1>; + #phy-cells = <0>; + + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>; + clock-names = "iface_clk"; + }; + + dsi1: dsi@ae96000 { + compatible = "qcom,mdss-dsi-ctrl"; + reg = <0xae96000 0x400>; + reg-names = "dsi_ctrl"; + + interrupt-parent = <&mdss>; + interrupts = <5 0>; + + clocks = <&dispcc DISP_CC_MDSS_BYTE1_CLK>, + <&dispcc DISP_CC_MDSS_BYTE1_INTF_CLK>, + <&dispcc DISP_CC_MDSS_PCLK1_CLK>, + <&dispcc DISP_CC_MDSS_ESC1_CLK>, + <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&dispcc DISP_CC_MDSS_AXI_CLK>; + clock-names = "byte_clk", + "byte_intf_clk", + "pixel_clk", + "core_clk", + "iface_clk", + "bus_clk"; + + phys = <&dsi1_phy>; + phy-names = "dsi-phy"; + + #address-cells = <1>; + #size-cells = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dsi1_in: endpoint { + remote-endpoint = <&dpu_intf2_out>; + }; + }; + + port@1 { + reg = <1>; + dsi1_out: endpoint { + }; + }; + }; + }; + + dsi1_phy: dsi-phy@ae96400 { + compatible = "qcom,dsi-phy-10nm"; + reg = <0xae96400 0x200>, + <0xae96a00 0x10e>, + <0xae96600 0x280>; + reg-names = "dsi_phy", + "dsi_pll", + "dsi_phy_lane"; + + #clock-cells = <1>; + #phy-cells = <0>; + + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>; + clock-names = "iface_clk"; + }; + }; + + dispcc: qcom,dispcc@af00000 { + compatible = "qcom,sdm845-dispcc"; + reg = <0xaf00000 0x10000>; + #clock-cells = <1>; + #reset-cells = <1>; + #power-domain-cells = <1>; + }; + spmi_bus: spmi@c440000 { compatible = "qcom,spmi-pmic-arb"; reg = <0xc440000 0x1100>,

On Tue, Jul 10, 2018 at 11:45 AM, Sean Paul seanpaul@chromium.org wrote:

The patch was rejected for being too big. Please refer to https://gitlab.freedesktop.org/seanpaul/dpu-staging/commit/b4215cf040d197828...

heh, and also seems to be too big for gmail to reply to..

That said, +30k is a nice improvement over the +110k where this started. But a few comments..

+static void dpu_kms_preclose(struct msm_kms *kms, struct drm_file *file) +{ + struct dpu_kms *dpu_kms = to_dpu_kms(kms); + struct drm_device *dev = dpu_kms->dev; + struct msm_drm_private *priv = dev->dev_private; + unsigned int i; + + for (i = 0; i < priv->num_crtcs; i++) + dpu_crtc_cancel_pending_flip(priv->crtcs[i], file); +}

So, not totally a fan of bringing back preclose() from the dead, esp. since it is only supposed to be used by legacy drivers. Since all this does is fire off pending events, I think it could be dropped entirely... drm_events_release() should unlink unsent events from the drm_file, and if that isn't working properly, that seems like a bug that should be fixed in drm core.

+static int _dpu_core_irq_enable(struct dpu_kms *dpu_kms, int irq_idx) +{ + unsigned long irq_flags; + int ret = 0, enable_count; + + if (!dpu_kms || !dpu_kms->hw_intr || + !dpu_kms->irq_obj.enable_counts || + !dpu_kms->irq_obj.irq_counts) { + DPU_ERROR("invalid params\n"); + return -EINVAL; + } + + if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->irq_idx_tbl_size) { + DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx); + return -EINVAL; + } ... +int dpu_core_irq_enable(struct dpu_kms *dpu_kms, int *irq_idxs, u32 irq_count) +{ + int i, ret = 0, counts; + + if (!dpu_kms || !irq_idxs || !irq_count) { + DPU_ERROR("invalid params\n"); + return -EINVAL; + } ...

there is a *whole* lot of multiple levels of null checks like this for things that are only ever called internally (ie. not something that can be triggered by parameters passed from userspace).. which is really unnecessary.

(maybe something that someone who knows coccinelle better than I, could automate cleaning up?)

+/** + * dpu_core_irq_disable_nolock - disable core interrupt given by the index + * without lock + * @dpu_kms: Pointer to dpu kms context + * @irq_idx: interrupt index + */ +int dpu_core_irq_disable_nolock(struct dpu_kms *dpu_kms, int irq_idx) +{

appears to be unused? dpu_core_irq_read_nolock() also seems unused, and maybe some others.

diff --git a/include/uapi/media/msm_media_info.h b/include/uapi/media/msm_media_info.h new file mode 100644 index 000000000000..4f12e5c534c8 --- /dev/null +++ b/include/uapi/media/msm_media_info.h @@ -0,0 +1,1376 @@ +#ifndef __MEDIA_INFO_H__ +#define __MEDIA_INFO_H__ + ...

Not quite sure where this belongs, but (at least for now) include/uapi/media seems very wrong.

Maybe just move into drm/msm for now? We can always move it back to include/uapi later (but the other direction isn't so cool)

------------------

Anyways, if I notice anything else to complain about, I'll send more replies. Overall it looks much better than where we started, and I'm open to the idea of pulling it in to msm-next soon. I think moving msm_media_info.h out of uapi, and probably nuking preclose, should happen first.

BR, -R