On Thu, Feb 27, 2020 at 01:54:33PM -0800, Matthias Kaehlcke wrote:
On Mon, Dec 02, 2019 at 01:48:57PM +0000, 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.
changes in v2: -- Update copyright markings on all relevant files. -- Change pr_err() to DRM_ERROR() -- Use APIs directly instead of function pointers. -- Use drm_display_mode structure to store link parameters in the driver. -- Use macros for register definitions instead of hardcoded values. -- Replace writel_relaxed/readl_relaxed with writel/readl and remove memory barriers. -- Remove unnecessary NULL checks. -- Use drm helper functions for dpcd read/write. -- Use DRM_DEBUG_DP for debug msgs.
changes in V3: -- Removed changes in dpu_io_util.[ch] -- Added locking around "is_connected" flag and removed atomic_set() -- Removed the argument validation checks in all the static functions except initialization functions and few API calls across msm/dp files -- Removed hardcoded values for register reads/writes -- Removed vreg related generic structures. -- Added return values where ever necessary. -- Updated dp_ctrl_on function. -- Calling the ctrl specific catalog functions directly instead of function pointers. -- Added seperate change that adds standard value in drm_dp_helper file. -- Added separate change in this list that is used to initialize displayport in DPU driver. -- Added change to use drm_dp_get_adjust_request_voltage() function.
Signed-off-by: Chandan Uddaraju chandanu@codeaurora.org
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
...
+int dp_power_init(struct dp_power *dp_power, bool flip) +{
- int rc = 0;
- struct dp_power_private *power;
- if (!dp_power) {
DRM_ERROR("invalid power data\n");rc = -EINVAL;goto exit;- }
drive-by comment:
this would lead to calling 'pm_runtime_put_sync(&power->pdev->dev)' below with 'power' being NULL, which doesn't seem a good idea.
correction: with 'power' being uninitialized, which isn't a good idea either.
It is probably sane to expect that 'dp_power' is not NULL, if that's the case the check can be removed. Otherwise the function should just return -EINVAL instead of jumping to 'exit'.
- power = container_of(dp_power, struct dp_power_private, dp_power);
- pm_runtime_get_sync(&power->pdev->dev);
- rc = dp_power_regulator_enable(power);
- if (rc) {
DRM_ERROR("failed to enable regulators, %d\n", rc);goto exit;- }
- rc = dp_power_pinctrl_set(power, true);
- if (rc) {
DRM_ERROR("failed to set pinctrl state, %d\n", rc);goto err_pinctrl;- }
- rc = dp_power_config_gpios(power, flip);
- if (rc) {
DRM_ERROR("failed to enable gpios, %d\n", rc);goto err_gpio;- }
- rc = dp_power_clk_enable(dp_power, DP_CORE_PM, true);
- if (rc) {
DRM_ERROR("failed to enable DP core clocks, %d\n", rc);goto err_clk;- }
- return 0;
+err_clk:
- dp_power_disable_gpios(power);
+err_gpio:
- dp_power_pinctrl_set(power, false);
+err_pinctrl:
- dp_power_regulator_disable(power);
+exit:
- pm_runtime_put_sync(&power->pdev->dev);
- return rc;
+}