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@xxxxxxxxxxxxxx>
> ---
> +++ 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.
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;
> +}
