Re: [Freedreno] [DPU PATCH 2/3] drm/msm/dp: add displayPort driver support

2019-03-09 Thread chandanu

On 2019-02-14 04:28, chand...@codeaurora.org wrote:
Hello Sean
I had few more queries regarding your comments. Can you please provide 
your feedback? (Queries/responses are present below)


thanks
Chandan


On 2018-10-23 09:28, Sean Paul wrote:

On Wed, Oct 10, 2018 at 10:15:58AM -0700, Chandan Uddaraju wrote:

Add the needed displayPort files to enable DP driver
on msm target.

"dp_display" module is the main module that calls into
other sub-modules. "dp_drm" file represents the interface
between DRM framework and DP driver.





Hello Sean,
I have responded to all of your comments and queries. I have specified
the comments addressed
in V2 patch series and what I have missed that will be fixed in V3.



Hi Chandan,
A few themes which I've pointed out below, but want to call out at the 
top so

it's obvious.

- Please remove all superfluous NULL checks at the beginning of 
functions.
  Included in this is reflowing initialization of variables and 
checking that
  return types still make sense (ie: if a function's only error path 
is the NULL

  check, make it void).
- Please remove all of the new callbacks and just call functions 
directly. This
  should also get rid of the _get/_put functions that allocate/set 
them up
- Please consolidate state structs into one struct with a few 
substructs,

  there's too much state right now.
- Please #define hardcoded values
- Instead of sprinkling wmb() everywhere, use readl/writel instead of 
*_relaxed
- There are a bunch of races that I found, please audit the code and 
remove

  races with locks as necessary
- I tried to catch as much dead code as possible, please audit the 
rest for

  unused structs/variables/functions/etc
- Use drm dp helper structs for link parameter storage



Signed-off-by: Chandan Uddaraju 
---
 drivers/gpu/drm/msm/Kconfig |9 +
 drivers/gpu/drm/msm/Makefile|   15 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c |  206 
 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h |   44 +
 drivers/gpu/drm/msm/dp/dp_aux.c |  570 ++
 drivers/gpu/drm/msm/dp/dp_aux.h |   44 +
 drivers/gpu/drm/msm/dp/dp_catalog.c | 1188 


 drivers/gpu/drm/msm/dp/dp_catalog.h |  144 +++
 drivers/gpu/drm/msm/dp/dp_ctrl.c| 1475 
+

 drivers/gpu/drm/msm/dp/dp_ctrl.h|   50 +
 drivers/gpu/drm/msm/dp/dp_debug.c   |  507 +
 drivers/gpu/drm/msm/dp/dp_debug.h   |   81 ++
 drivers/gpu/drm/msm/dp/dp_display.c |  977 +
 drivers/gpu/drm/msm/dp/dp_display.h |   55 +
 drivers/gpu/drm/msm/dp/dp_drm.c |  542 ++
 drivers/gpu/drm/msm/dp/dp_drm.h |   52 +
 drivers/gpu/drm/msm/dp/dp_extcon.c  |  400 +++
 drivers/gpu/drm/msm/dp/dp_extcon.h  |  111 ++
 drivers/gpu/drm/msm/dp/dp_link.c| 1549 
+++

 drivers/gpu/drm/msm/dp/dp_link.h|  184 
 drivers/gpu/drm/msm/dp/dp_panel.c   |  624 +++
 drivers/gpu/drm/msm/dp/dp_panel.h   |  121 +++
 drivers/gpu/drm/msm/dp/dp_parser.c  |  679 
 drivers/gpu/drm/msm/dp/dp_parser.h  |  205 
 drivers/gpu/drm/msm/dp/dp_power.c   |  599 +++
 drivers/gpu/drm/msm/dp/dp_power.h   |   57 +
 drivers/gpu/drm/msm/dp/dp_reg.h |  357 ++
 drivers/gpu/drm/msm/msm_drv.c   |2 +
 drivers/gpu/drm/msm/msm_drv.h   |   22 +
 include/drm/drm_dp_helper.h |   19 +
 30 files changed, 10887 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_display.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_display.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_extcon.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_extcon.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_link.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_link.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_power.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_power.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_reg.h





diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c 
b/drivers/gpu/drm/msm/dp/dp_catalog.c

new file mode 100644
ind

Re: [Freedreno] [DPU PATCH 2/3] drm/msm/dp: add displayPort driver support

2019-02-14 Thread Sean Paul
On Thu, Feb 14, 2019 at 04:28:33AM -0800, chand...@codeaurora.org wrote:
> On 2018-10-23 09:28, Sean Paul wrote:
> > On Wed, Oct 10, 2018 at 10:15:58AM -0700, Chandan Uddaraju wrote:
> > > Add the needed displayPort files to enable DP driver
> > > on msm target.
> > > 
> > > "dp_display" module is the main module that calls into
> > > other sub-modules. "dp_drm" file represents the interface
> > > between DRM framework and DP driver.
> > > 
> > 
> 
> Hello Sean,
> I have responded to all of your comments and queries. I have specified the
> comments addressed
> in V2 patch series and what I have missed that will be fixed in V3.
> 
> 

/snip

> > > 
> > > +struct dss_io_data {
> > > + u32 len;
> > > + void __iomem *base;
> > > +};
> > 
> > len isn't used anywhere, so this struct can go away. If the struct goes
> > away,
> > you can really simplify (or eliminate msm_dss_ioremap_byname())
> > 
> 
> We want to keep this since we use len for our internal unit level testing
> and in simulation mode.
> 

That's not really a valid reason to keep unused code around. We can't reasonably
expect upstream developers to know which structs and unused fields are used for
Qualcomm's downstream testing, and afaict, those results aren't published so
there's no benefit to the upsteam community.

So you can either opensource the unit tests along with the driver, or keep the
downstream code in the downstream kernel.

/snip

> > > +
> > > +#define dp_catalog_get_priv(x) { \
> > 
> > static inline struct dp_catalog_private *dp_catalog_get_priv(...)
> > {
> > ...
> > }
> > 
> For inline, we have to use 3 different functions (one each for aux, ctrl and
> panel).
> Looking at "type-checking" w.r.t macro, container_of will through an error
> if its not valid pointer.
> Please let me know if you still want to change this macro to inline func.
> 

Yeah, I think it's worth it to have the 3.

> > > + struct dp_catalog *dp_catalog; \
> > > + dp_catalog = container_of(x, struct dp_catalog, x); \
> > > + catalog = container_of(dp_catalog, struct dp_catalog_private, \
> > > + dp_catalog); \
> > > +}

/snip

> > 
> > Then again, I'm guessing hitting the NULL case is likely impossible, so
> > you can
> > probably remove all of the machinery around checking !aux.
> > 
> 
> Removed most of the arg null checks except few APIs where
> unit-testing/simulator-mode is used.
> 

There should only be null checks if null is a possibility. If you have
pathelogical unit tests injecting bogus values in your function, you should
probably just fix/delete those unit tests. If upstream doesn't have
visibility into your internal testing, we can't distinguish between
test-pleasing vs unused code.

> > > +static void dp_catalog_ctrl_send_phy_pattern(struct dp_catalog_ctrl
> > > *ctrl,
> > > + u32 pattern)
> > > +{
> > > + struct dp_catalog_private *catalog;
> > > + u32 value = 0x0;
> > > + void __iomem *base = NULL;
> > > +
> > > + if (!ctrl) {
> > > + pr_err("invalid input\n");
> > > + return;
> > > + }
> > > +
> > > + dp_catalog_get_priv(ctrl);
> > > +
> > > + base = catalog->io->dp_link.base;
> > > +
> > > + dp_write(base + DP_STATE_CTRL, 0x0);
> > 
> > Do you really need to do this?
> > 
> Yes, to make sure no other pattern is set/enabled in hardware before any new
> pattern is configured.
> 
> > > +
> > > + switch (pattern) {
> > > + case DP_TEST_PHY_PATTERN_D10_2_NO_SCRAMBLING:
> > > + dp_write(base + DP_STATE_CTRL, 0x1);
> > > + break;
> > > + case DP_TEST_PHY_PATTERN_SYMBOL_ERR_MEASUREMENT_CNT:
> > > + value &= ~(1 << 16);
> > > + dp_write(base + DP_HBR2_COMPLIANCE_SCRAMBLER_RESET, value);
> > > + value |= 0xFC;
> > > + dp_write(base + DP_HBR2_COMPLIANCE_SCRAMBLER_RESET, value);
> > > + dp_write(base + DP_MAINLINK_LEVELS, 0x2);
> > > + dp_write(base + DP_STATE_CTRL, 0x10);
> > > + break;
> > > + case DP_TEST_PHY_PATTERN_PRBS7:
> > > + dp_write(base + DP_STATE_CTRL, 0x20);
> > > + break;
> > > + case DP_TEST_PHY_PATTERN_80_BIT_CUSTOM_PATTERN:
> > > + dp_write(base + DP_STATE_CTRL, 0x40);
> > > + /* 00101010 */
> > > + dp_write(base + DP_TEST_80BIT_CUSTOM_PATTERN_REG0, 0x3E0F83E0);
> > > + /* 10101000 */
> > > + dp_write(base + DP_TEST_80BIT_CUSTOM_PATTERN_REG1, 0x0F83E0F8);
> > > + /* 1010 */
> > > + dp_write(base + DP_TEST_80BIT_CUSTOM_PATTERN_REG2, 0xF83E);
> > > + break;
> > > + case DP_TEST_PHY_PATTERN_HBR2_CTS_EYE_PATTERN:
> > > + value = BIT(16);
> > > + dp_write(base + DP_HBR2_COMPLIANCE_SCRAMBLER_RESET, value);
> > > + value |= 0xFC;
> > > + dp_write(base + DP_HBR2_COMPLIANCE_SCRAMBLER_RESET, value);
> > > + dp_write(base + DP_MAINLINK_LEVELS, 0x2);
> > > + dp_write(base + DP_STATE_CTRL, 0x10);
> > 
> > This code is the exact s

Re: [Freedreno] [DPU PATCH 2/3] drm/msm/dp: add displayPort driver support

2018-11-26 Thread Sean Paul
On Mon, Nov 19, 2018 at 02:34:53PM -0800, chand...@codeaurora.org wrote:
> On 2018-10-23 09:28, Sean Paul wrote:
> > On Wed, Oct 10, 2018 at 10:15:58AM -0700, Chandan Uddaraju wrote:
> > > Add the needed displayPort files to enable DP driver
> > > on msm target.
> > > 
> > > "dp_display" module is the main module that calls into
> > > other sub-modules. "dp_drm" file represents the interface
> > > between DRM framework and DP driver.
> > > 
> > 
> 
> Hello Sean,
> I had few queries regarding your comments. Shared my queries below. Please
> share your feedback.

/snip

> > > diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c
> > > b/drivers/gpu/drm/msm/dp/dp_aux.c
> > > new file mode 100644
> > > index 000..00b82c2
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/msm/dp/dp_aux.c
> > > @@ -0,0 +1,570 @@
> > 
> > /snip
> > 
> > > diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h
> > > b/drivers/gpu/drm/msm/dp/dp_aux.h
> > > new file mode 100644
> > > index 000..0c300ed
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/msm/dp/dp_aux.h
> > 
> > /snip
> > 
> > I didn't really review the dp_aux.* files as-is. Please convert to using
> > drm_dp_aux and I'll take another look.
> > 
> 
> Please correct me if I am wrong. Looks like you are suggesting to use most
> of the drm_dp_aux struct variable
> instead of what we have defined locally.
> 

Correct


/snip


> > > + struct dp_catalog_ctrl ctrl = {
> > > + .state_ctrl = dp_catalog_ctrl_state_ctrl,
> > > + .config_ctrl= dp_catalog_ctrl_config_ctrl,
> > > + .lane_mapping   = dp_catalog_ctrl_lane_mapping,
> > > + .mainlink_ctrl  = dp_catalog_ctrl_mainlink_ctrl,
> > > + .config_misc= dp_catalog_ctrl_config_misc,
> > > + .config_msa = dp_catalog_ctrl_config_msa,
> > > + .set_pattern= dp_catalog_ctrl_set_pattern,
> > > + .reset  = dp_catalog_ctrl_reset,
> > > + .usb_reset  = dp_catalog_ctrl_usb_reset,
> > > + .mainlink_ready = dp_catalog_ctrl_mainlink_ready,
> > > + .enable_irq = dp_catalog_ctrl_enable_irq,
> > > + .hpd_config = dp_catalog_ctrl_hpd_config,
> > > + .phy_reset  = dp_catalog_ctrl_phy_reset,
> > > + .phy_lane_cfg   = dp_catalog_ctrl_phy_lane_cfg,
> > > + .update_vx_px   = dp_catalog_ctrl_update_vx_px,
> > > + .get_interrupt  = dp_catalog_ctrl_get_interrupt,
> > > + .update_transfer_unit = dp_catalog_ctrl_update_transfer_unit,
> > > + .send_phy_pattern= dp_catalog_ctrl_send_phy_pattern,
> > > + .read_phy_pattern = dp_catalog_ctrl_read_phy_pattern,
> > 
> > So, what's the benefit of adding the catalog abstractions? Do you ever
> > have
> > multiple/alternate implementations of a catalog? It doesn't seem like
> > it. You
> > should just remove all of the catalog overhead and call the functions
> > directly.
> 
> We use catalog abstractions for two different reasons:
> 1. To support multiple hardware since these implementations are hardware
> specific.
> 2. To virtualize DP hardware.
> 
> Our internal hardware already supports multiple chips. We want to keep these
> abstractions to
> make the code consistent between upstream and internal implementation.
> 

I'm not sure how this differs from having a common header/function declarations
and multiple definitions in a .c gated by CONFIG_*? Ideally even this wouldn't 
be
necessary since hw changes are either incremental and can be done more precisely
than wholesale changing the implementation.

If you want to virtualize, you can do the CONFIG_* thing. However since that's
not going upstream, you can carry the Kconfig/Makefile change in your
downstream.

All of these abstractions add thousands of lines of code, extra files, and 
multiple
levels of indirection. IMO, I don't think the benefits outweigh the costs.

/snip

> > > +static void dp_ctrl_calc_tu_parameters(struct dp_ctrl_private *ctrl,
> > > +struct dp_vc_tu_mapping_table *tu_table)
> > > +{
> > > + u32 multiplier = 100;
> > > + u64 pclk, lclk;
> > > + u8 bpp, ln_cnt;
> > > + int run_idx = 0;
> > > + u32 lwidth, h_blank;
> > > + u32 fifo_empty = 0;
> > > + u32 ratio_scale = 1001;
> > > + u64 temp, ratio, original_ratio;
> > > + u64 temp2, reminder;
> > > + u64 temp3, temp4, result = 0;
> > > +
> > > + u64 err = multiplier;
> > > + u64 n_err = 0, n_n_err = 0;
> > > + bool n_err_neg, nn_err_neg;
> > > + u8 hblank_margin = 16;
> > > +
> > > + u8 tu_size, tu_size_desired = 0, tu_size_minus1;
> > > + int valid_boundary_link;
> > > + u64 resulting_valid;
> > > + u64 total_valid;
> > > + u64 effective_valid;
> > > + u64 effective_valid_recorded;
> > > + int n_tus;
> > > + int n_tus_per_lane;
> > > + int paired_tus;
> > > + int remainder_tus;
> > > + int remainder_tus_upper, remainder_tus_lower;
> > > + int extra_bytes;
> > > + int filler_size;
> > > + int delay_start_link;
> > > + int boundary_moderation_en = 0;
> > > + int upper_bdry_cnt =

Re: [Freedreno] [DPU PATCH 2/3] drm/msm/dp: add displayPort driver support

2018-10-11 Thread Jordan Crouse
On Wed, Oct 10, 2018 at 10:15:58AM -0700, Chandan Uddaraju wrote:
> Add the needed displayPort files to enable DP driver
> on msm target.
> 
> "dp_display" module is the main module that calls into
> other sub-modules. "dp_drm" file represents the interface
> between DRM framework and DP driver.
> 
> Signed-off-by: Chandan Uddaraju 
> ---
>  drivers/gpu/drm/msm/Kconfig |9 +
>  drivers/gpu/drm/msm/Makefile|   15 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c |  206 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h |   44 +
>  drivers/gpu/drm/msm/dp/dp_aux.c |  570 ++
>  drivers/gpu/drm/msm/dp/dp_aux.h |   44 +
>  drivers/gpu/drm/msm/dp/dp_catalog.c | 1188 
>  drivers/gpu/drm/msm/dp/dp_catalog.h |  144 +++
>  drivers/gpu/drm/msm/dp/dp_ctrl.c| 1475 +
>  drivers/gpu/drm/msm/dp/dp_ctrl.h|   50 +
>  drivers/gpu/drm/msm/dp/dp_debug.c   |  507 +
>  drivers/gpu/drm/msm/dp/dp_debug.h   |   81 ++
>  drivers/gpu/drm/msm/dp/dp_display.c |  977 +
>  drivers/gpu/drm/msm/dp/dp_display.h |   55 +
>  drivers/gpu/drm/msm/dp/dp_drm.c |  542 ++
>  drivers/gpu/drm/msm/dp/dp_drm.h |   52 +
>  drivers/gpu/drm/msm/dp/dp_extcon.c  |  400 +++
>  drivers/gpu/drm/msm/dp/dp_extcon.h  |  111 ++
>  drivers/gpu/drm/msm/dp/dp_link.c| 1549 
> +++
>  drivers/gpu/drm/msm/dp/dp_link.h|  184 
>  drivers/gpu/drm/msm/dp/dp_panel.c   |  624 +++
>  drivers/gpu/drm/msm/dp/dp_panel.h   |  121 +++
>  drivers/gpu/drm/msm/dp/dp_parser.c  |  679 
>  drivers/gpu/drm/msm/dp/dp_parser.h  |  205 
>  drivers/gpu/drm/msm/dp/dp_power.c   |  599 +++
>  drivers/gpu/drm/msm/dp/dp_power.h   |   57 +
>  drivers/gpu/drm/msm/dp/dp_reg.h |  357 ++
>  drivers/gpu/drm/msm/msm_drv.c   |2 +
>  drivers/gpu/drm/msm/msm_drv.h   |   22 +
>  include/drm/drm_dp_helper.h |   19 +
>  30 files changed, 10887 insertions(+), 1 deletion(-)

This is my last go - the end is in sight.



> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c 
> b/drivers/gpu/drm/msm/dp/dp_parser.c
> new file mode 100644
> index 000..a366dc5
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -0,0 +1,679 @@
> +/*
> + * Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */

SPDX please

> +#define pr_fmt(fmt)  "[drm-dp] %s: " fmt, __func__
> +
> +#include 
> +
> +#include "dp_parser.h"
> +
> +static void dp_parser_unmap_io_resources(struct dp_parser *parser)
> +{
> + struct dp_io *io = &parser->io;
> +
> + msm_dss_iounmap(&io->dp_ahb);
> + msm_dss_iounmap(&io->dp_aux);
> + msm_dss_iounmap(&io->dp_link);
> + msm_dss_iounmap(&io->dp_p0);
> + msm_dss_iounmap(&io->phy_io);
> + msm_dss_iounmap(&io->ln_tx0_io);
> + msm_dss_iounmap(&io->ln_tx0_io);
> + msm_dss_iounmap(&io->dp_pll_io);
> + msm_dss_iounmap(&io->dp_cc_io);
> + msm_dss_iounmap(&io->usb3_dp_com);
> + msm_dss_iounmap(&io->qfprom_io);
> +}

If you use devm_ to ioremap as suggested previously, then this all ends up going
away.

> +static int dp_parser_ctrl_res(struct dp_parser *parser)
> +{
> + int rc = 0;
> + u32 index;
> + struct platform_device *pdev = parser->pdev;
> + struct device_node *of_node = parser->pdev->dev.of_node;
> + struct dp_io *io = &parser->io;
> +
> + rc = of_property_read_u32(of_node, "cell-index", &index);
> + if (rc) {
> + pr_err("cell-index not specified, rc=%d\n", rc);
> + goto err;
> + }

This value doesn't appear to be used anywhere in this code, so why are you
grabbing it?

> + rc = msm_dss_ioremap_byname(pdev, &io->dp_ahb, "dp_ahb");
> + if (rc) {
> + pr_err("unable to remap dp io resources\n");

msm_ioremap() helpfully gives an informative error message including the name of
the region that failed  so you don't need a redundant message here.

> + goto err;
> + }
> +
> + rc = msm_dss_ioremap_byname(pdev, &io->dp_aux, "dp_aux");
> + if (rc) {
> + pr_err("unable to remap dp io resources\n");
> + goto err;
> + }
> +
> + rc = msm_dss_ioremap_byname(pdev, &io->dp_link, "dp_link");
> + if 

Re: [Freedreno] [DPU PATCH 2/3] drm/msm/dp: add displayPort driver support

2018-10-10 Thread Jordan Crouse
On Wed, Oct 10, 2018 at 10:15:58AM -0700, Chandan Uddaraju wrote:
> Add the needed displayPort files to enable DP driver
> on msm target.
> 
> "dp_display" module is the main module that calls into
> other sub-modules. "dp_drm" file represents the interface
> between DRM framework and DP driver.
> 
> Signed-off-by: Chandan Uddaraju 
> ---
>  drivers/gpu/drm/msm/Kconfig |9 +
>  drivers/gpu/drm/msm/Makefile|   15 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c |  206 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h |   44 +
>  drivers/gpu/drm/msm/dp/dp_aux.c |  570 ++
>  drivers/gpu/drm/msm/dp/dp_aux.h |   44 +
>  drivers/gpu/drm/msm/dp/dp_catalog.c | 1188 
>  drivers/gpu/drm/msm/dp/dp_catalog.h |  144 +++
>  drivers/gpu/drm/msm/dp/dp_ctrl.c| 1475 +
>  drivers/gpu/drm/msm/dp/dp_ctrl.h|   50 +
>  drivers/gpu/drm/msm/dp/dp_debug.c   |  507 +
>  drivers/gpu/drm/msm/dp/dp_debug.h   |   81 ++
>  drivers/gpu/drm/msm/dp/dp_display.c |  977 +
>  drivers/gpu/drm/msm/dp/dp_display.h |   55 +
>  drivers/gpu/drm/msm/dp/dp_drm.c |  542 ++
>  drivers/gpu/drm/msm/dp/dp_drm.h |   52 +
>  drivers/gpu/drm/msm/dp/dp_extcon.c  |  400 +++
>  drivers/gpu/drm/msm/dp/dp_extcon.h  |  111 ++
>  drivers/gpu/drm/msm/dp/dp_link.c| 1549 
> +++
>  drivers/gpu/drm/msm/dp/dp_link.h|  184 
>  drivers/gpu/drm/msm/dp/dp_panel.c   |  624 +++
>  drivers/gpu/drm/msm/dp/dp_panel.h   |  121 +++
>  drivers/gpu/drm/msm/dp/dp_parser.c  |  679 
>  drivers/gpu/drm/msm/dp/dp_parser.h  |  205 
>  drivers/gpu/drm/msm/dp/dp_power.c   |  599 +++
>  drivers/gpu/drm/msm/dp/dp_power.h   |   57 +
>  drivers/gpu/drm/msm/dp/dp_reg.h |  357 ++
>  drivers/gpu/drm/msm/msm_drv.c   |2 +
>  drivers/gpu/drm/msm/msm_drv.h   |   22 +
>  include/drm/drm_dp_helper.h |   19 +
>  30 files changed, 10887 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_display.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_display.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_extcon.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_extcon.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_link.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_link.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_power.c
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_power.h
>  create mode 100644 drivers/gpu/drm/msm/dp/dp_reg.h
> 
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index 843a9d4..c363f24 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -49,6 +49,15 @@ config DRM_MSM_HDMI_HDCP
>   help
> Choose this option to enable HDCP state machine
>  
> +config DRM_MSM_DP
> + bool "Enable DP support in MSM DRM driver"
> + depends on DRM_MSM
> + default n

default n should be implied so you don't need to add it.

> + help
> +   Compile in support for DP driver in msm drm
> +   driver. DP external display support is enabled
> +   through this config option. It can be primary or
> +   secondary display on device.
>  config DRM_MSM_DSI
>   bool "Enable DSI support in MSM DRM driver"
>   depends on DRM_MSM
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 19ab521..765a8d8 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -2,6 +2,7 @@
>  ccflags-y := -Idrivers/gpu/drm/msm
>  ccflags-y += -Idrivers/gpu/drm/msm/disp/dpu1
>  ccflags-$(CONFIG_DRM_MSM_DSI) += -Idrivers/gpu/drm/msm/dsi
> +ccflags-$(CONFIG_DRM_MSM_DP) += -Idrivers/gpu/drm/msm/dp
>  
>  msm-y := \
>   adreno/adreno_device.o \
> @@ -93,7 +94,19 @@ msm-y := \
>   msm_submitqueue.o
>  
>  msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \
> -   disp/dpu1/dpu_dbg.o
> +   disp/dpu1/dpu_dbg.o