mbox series

[v10,0/5] Add support for DisplayPort driver on SnapDragon

Message ID 20200812044223.19279-1-tanmay@codeaurora.org (mailing list archive)
Headers show
Series Add support for DisplayPort driver on SnapDragon | expand

Message

Tanmay Shah Aug. 12, 2020, 4:42 a.m. UTC
These patches add Display-Port driver on SnapDragon/msm hardware.
This series also contains device-tree bindings for msm DP driver.
It also contains Makefile and Kconfig changes to compile msm DP driver.

The block diagram of DP driver is shown below:


                 +-------------+
                 |DRM FRAMEWORK|
                 +------+------+
                        |
                   +----v----+
                   | DP DRM  |
                   +----+----+
                        |
                   +----v----+
     +------------+|   DP    +----------++------+
     +        +---+| DISPLAY |+---+      |      |
     |        +    +-+-----+-+    |      |      |
     |        |      |     |      |      |      |
     |        |      |     |      |      |      |
     |        |      |     |      |      |      |
     v        v      v     v      v      v      v
 +------+ +------+ +---+ +----+ +----+ +---+ +-----+
 |  DP  | |  DP  | |DP | | DP | | DP | |DP | | DP  |
 |PARSER| | HPD  | |AUX| |LINK| |CTRL| |PHY| |POWER|
 +--+---+ +---+--+ +---+ +----+ +--+-+ +-+-+ +-----+
    |                              |     |
 +--v---+                         +v-----v+
 |DEVICE|                         |  DP   |
 | TREE |                         |CATALOG|
 +------+                         +---+---+
                                      |
                                  +---v----+
                                  |CTRL/PHY|
                                  |   HW   |
                                  +--------+

Changes in v7:

- Modify cover letter description and fix title.
- Introduce dp-controller.yaml for common bindings across SOC
- Rename dp-sc7180.yaml to dp-controller-sc7180.yaml for SC7180 bindings
- Rename compatible string to qcom,sc7180-dp
- Add assigned-clocks and assigned-clock-parents properties in bindings
- Remove redundant code from driver
- Extend series to include HPD detection logic

Changes in v8:

- Add MDSS AHB clock in bindings 
- Replace mode->vrefresh use with drm_mode_vrefresh API
- Remove redundant aux config code from parser and aux module
- Assign default max lanes if data-lanes property is not available
- Fix use-after-free during DP driver remove
- Unregister hardware clocks during driver cleanup

Changes in v9:

- Drop YAML bindings change from the series
- Use assigne-clock-parents property and remove clk_set_parent use from code
- Access register address space without name
- Fix DP register dump utility
- Disable DP clocks after vsync generated
- Avoid 64-bit modulo operation
- Drop any unused code and fix function proptotyes to avoid W=1 warnings
- Drop DRM_MSM_DP_10NM_PLL config as only 10nm PLL is available

Changes in v10:

- Fix help description of Kconfig entry

Chandan Uddaraju (4):
  dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon
  drm: add constant N value in helper file
  drm/msm/dp: add displayPort driver support
  drm/msm/dp: add support for DP PLL driver

Jeykumar Sankaran (1):
  drm/msm/dpu: add display port support in DPU

Tanmay Shah (1):
  drm/msm/dp: Add Display Port HPD feature

 drivers/gpu/drm/i915/display/intel_display.c  |    2 +-
 drivers/gpu/drm/msm/Kconfig                   |    9 +
 drivers/gpu/drm/msm/Makefile                  |   14 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |   27 +-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |    8 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |   83 +-
 drivers/gpu/drm/msm/dp/dp_aux.c               |  510 +++++
 drivers/gpu/drm/msm/dp/dp_aux.h               |   29 +
 drivers/gpu/drm/msm/dp/dp_catalog.c           | 1030 ++++++++++
 drivers/gpu/drm/msm/dp/dp_catalog.h           |  104 +
 drivers/gpu/drm/msm/dp/dp_ctrl.c              | 1693 +++++++++++++++++
 drivers/gpu/drm/msm/dp/dp_ctrl.h              |   35 +
 drivers/gpu/drm/msm/dp/dp_display.c           | 1017 ++++++++++
 drivers/gpu/drm/msm/dp/dp_display.h           |   31 +
 drivers/gpu/drm/msm/dp/dp_drm.c               |  168 ++
 drivers/gpu/drm/msm/dp/dp_drm.h               |   18 +
 drivers/gpu/drm/msm/dp/dp_hpd.c               |   69 +
 drivers/gpu/drm/msm/dp/dp_hpd.h               |   79 +
 drivers/gpu/drm/msm/dp/dp_link.c              | 1214 ++++++++++++
 drivers/gpu/drm/msm/dp/dp_link.h              |  132 ++
 drivers/gpu/drm/msm/dp/dp_panel.c             |  486 +++++
 drivers/gpu/drm/msm/dp/dp_panel.h             |   95 +
 drivers/gpu/drm/msm/dp/dp_parser.c            |  269 +++
 drivers/gpu/drm/msm/dp/dp_parser.h            |  138 ++
 drivers/gpu/drm/msm/dp/dp_pll.c               |   99 +
 drivers/gpu/drm/msm/dp/dp_pll.h               |   61 +
 drivers/gpu/drm/msm/dp/dp_pll_10nm.c          |  917 +++++++++
 drivers/gpu/drm/msm/dp/dp_pll_private.h       |   98 +
 drivers/gpu/drm/msm/dp/dp_power.c             |  373 ++++
 drivers/gpu/drm/msm/dp/dp_power.h             |  103 +
 drivers/gpu/drm/msm/dp/dp_reg.h               |  517 +++++
 drivers/gpu/drm/msm/msm_drv.c                 |    2 +
 drivers/gpu/drm/msm/msm_drv.h                 |   59 +-
 include/drm/drm_dp_helper.h                   |    1 +
 34 files changed, 9471 insertions(+), 19 deletions(-)
 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_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_hpd.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_hpd.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_pll.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll.h
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_10nm.c
 create mode 100644 drivers/gpu/drm/msm/dp/dp_pll_private.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


base-commit: 418eda8f3fe292782c150266d693d55d284c0c98

Comments

Dmitry Baryshkov Aug. 14, 2020, 5:12 p.m. UTC | #1
Hello,

On 12/08/2020 07:42, Tanmay Shah wrote:
> From: Chandan Uddaraju <chandanu@codeaurora.org>

[skipped]

> +		} else if ((dp_parser_check_prefix("ctrl", clk_name) ||
> +			   dp_parser_check_prefix("stream", clk_name))  &&
> +			   ctrl_clk_index < ctrl_clk_count) {
> +			struct dss_clk *clk =
> +				&ctrl_power->clk_config[ctrl_clk_index];
> +			strlcpy(clk->clk_name, clk_name, sizeof(clk->clk_name));
> +			ctrl_clk_index++;
> +
> +			if (!strncmp(clk_name, "ctrl_link",
> +					strlen("ctrl_link")) ||
> +					!strncmp(clk_name, "stream_pixel",
> +					strlen("ctrl_pixel")))

This should be "stream_pixel", I believe. I don't like macros, but most 
probably it would help here. Also function/brace alignment could be 
better (sorry, it really hides the issue here).


> +				clk->type = DSS_CLK_PCLK;
> +			else
> +				clk->type = DSS_CLK_AHB;
> +		}
> +	}
Tanmay Shah Aug. 14, 2020, 5:56 p.m. UTC | #2
On 2020-08-14 10:12, Dmitry Baryshkov wrote:
> Hello,
> 
> On 12/08/2020 07:42, Tanmay Shah wrote:
>> From: Chandan Uddaraju <chandanu@codeaurora.org>
> 
> [skipped]
> 
>> +		} else if ((dp_parser_check_prefix("ctrl", clk_name) ||
>> +			   dp_parser_check_prefix("stream", clk_name))  &&
>> +			   ctrl_clk_index < ctrl_clk_count) {
>> +			struct dss_clk *clk =
>> +				&ctrl_power->clk_config[ctrl_clk_index];
>> +			strlcpy(clk->clk_name, clk_name, sizeof(clk->clk_name));
>> +			ctrl_clk_index++;
>> +
>> +			if (!strncmp(clk_name, "ctrl_link",
>> +					strlen("ctrl_link")) ||
>> +					!strncmp(clk_name, "stream_pixel",
>> +					strlen("ctrl_pixel")))
> 
> This should be "stream_pixel", I believe. I don't like macros, but
> most probably it would help here. Also function/brace alignment could
> be better (sorry, it really hides the issue here).
> 

Thanks for reviews and good catch!! I completely missed it when I 
renamed "ctrl_pixel".
Use of "stream_pixel" is very limited. So, instead of macros direct name 
is used.
Fixing function and brace alignment sounds good idea insted.

> 

>> +				clk->type = DSS_CLK_PCLK;
>> +			else
>> +				clk->type = DSS_CLK_AHB;
>> +		}
>> +	}
Tanmay Shah Aug. 14, 2020, 9:14 p.m. UTC | #3
On 2020-08-14 10:56, Tanmay Shah wrote:
> On 2020-08-14 10:12, Dmitry Baryshkov wrote:
>> Hello,
>> 
>> On 12/08/2020 07:42, Tanmay Shah wrote:
>>> From: Chandan Uddaraju <chandanu@codeaurora.org>
>> 
>> [skipped]
>> 
>>> +		} else if ((dp_parser_check_prefix("ctrl", clk_name) ||
>>> +			   dp_parser_check_prefix("stream", clk_name))  &&
>>> +			   ctrl_clk_index < ctrl_clk_count) {
>>> +			struct dss_clk *clk =
>>> +				&ctrl_power->clk_config[ctrl_clk_index];
>>> +			strlcpy(clk->clk_name, clk_name, sizeof(clk->clk_name));
>>> +			ctrl_clk_index++;
>>> +
>>> +			if (!strncmp(clk_name, "ctrl_link",
>>> +					strlen("ctrl_link")) ||
>>> +					!strncmp(clk_name, "stream_pixel",
>>> +					strlen("ctrl_pixel")))
>> 
>> This should be "stream_pixel", I believe. I don't like macros, but
>> most probably it would help here. Also function/brace alignment could
>> be better (sorry, it really hides the issue here).
>> 
> 
> Thanks for reviews and good catch!! I completely missed it when I
> renamed "ctrl_pixel".
> Use of "stream_pixel" is very limited. So, instead of macros direct
> name is used.
> Fixing function and brace alignment sounds good idea insted.
> 
>> 
Actually I will reuse dp_parser_check_prefix utility. It's already doing 
same Job.

> 
>>> +				clk->type = DSS_CLK_PCLK;
>>> +			else
>>> +				clk->type = DSS_CLK_AHB;
>>> +		}
>>> +	}
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel