linux-kernel - Re: [PATCH v10 4/4] drm/msm/dp: enable widebus feature for display port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Fri, 25 Feb 2022 10:17:50 -0800
From:   Stephen Boyd <swboyd@...omium.org>
To:     Kuogee Hsieh <quic_khsieh@...cinc.com>, agross@...nel.org,
        airlied@...ux.ie, bjorn.andersson@...aro.org, daniel@...ll.ch,
        dmitry.baryshkov@...aro.org, dri-devel@...ts.freedesktop.org,
        robdclark@...il.com, sean@...rly.run, vkoul@...nel.org
Cc:     quic_abhinavk@...cinc.com, quic_aravindh@...cinc.com,
        quic_sbillaka@...cinc.com, freedreno@...ts.freedesktop.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 4/4] drm/msm/dp: enable widebus feature for display port

Quoting Kuogee Hsieh (2022-02-22 16:27:40)
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index 6ae9b29..c789f4e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -483,6 +485,22 @@ int dp_catalog_ctrl_set_pattern(struct dp_catalog *dp_catalog,
>  }
>
>  /**
> + * dp_catalog_hw_revision() - retrieve DP hw revision
> + *
> + * @dp_catalog: DP catalog structure
> + *
> + * Return: DP controller hw revision
> + *
> + */
> +u32 dp_catalog_hw_revision(struct dp_catalog *dp_catalog)

Could be const

> +{
> +       struct dp_catalog_private *catalog = container_of(dp_catalog,
> +                               struct dp_catalog_private, dp_catalog);
> +
> +       return dp_read_ahb(catalog, REG_DP_HW_VERSION);

If dp_read_ahb() took a const catalog, which it could.

> +}
> +
> +/**
>   * dp_catalog_ctrl_reset() - reset DP controller
>   *
>   * @dp_catalog: DP catalog structure
> @@ -743,6 +761,7 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
>  {
>         struct dp_catalog_private *catalog = container_of(dp_catalog,
>                                 struct dp_catalog_private, dp_catalog);
> +       u32 reg;
>
>         dp_write_link(catalog, REG_DP_TOTAL_HOR_VER,
>                                 dp_catalog->total);
> @@ -751,7 +770,18 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog *dp_catalog)
>         dp_write_link(catalog, REG_DP_HSYNC_VSYNC_WIDTH_POLARITY,
>                                 dp_catalog->width_blanking);
>         dp_write_link(catalog, REG_DP_ACTIVE_HOR_VER, dp_catalog->dp_active);
> -       dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, 0);
> +
> +       reg = dp_read_p0(catalog, MMSS_DP_INTF_CONFIG);
> +
> +       if (dp_catalog->wide_bus_en)
> +               reg |= DP_INTF_CONFIG_DATABUS_WIDEN;
> +       else
> +               reg &= ~DP_INTF_CONFIG_DATABUS_WIDEN;
> +
> +
> +       DRM_DEBUG_DP("wide_bus_en=%d reg=%x\n", dp_catalog->wide_bus_en, reg);

Use %#x to get 0x prefix on the hex please.

> +
> +       dp_write_p0(catalog, MMSS_DP_INTF_CONFIG, reg);
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index 2363a2d..a0a5fbb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -17,6 +17,7 @@ struct dp_ctrl {
>         bool orientation;
>         atomic_t aborted;
>         u32 pixel_rate;
> +       bool wide_bus_en;
>  };
>
>  int dp_ctrl_host_init(struct dp_ctrl *dp_ctrl, bool flip, bool reset);
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 7cc4d21..ba76358 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1437,6 +1445,15 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
>         dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
>  }
>
> +bool msm_dp_wide_bus_available(struct msm_dp *dp_display)

const?

> +{
> +       struct dp_display_private *dp;
> +
> +       dp = container_of(dp_display, struct dp_display_private, dp_display);
> +
> +       return dp->wide_bus_en;
> +}
> +
>  void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)
>  {
>         struct dp_display_private *dp;
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
> index e3adcd5..b718cc9 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.h
> +++ b/drivers/gpu/drm/msm/dp/dp_display.h
> @@ -24,6 +24,8 @@ struct msm_dp {
>
>         hdmi_codec_plugged_cb plugged_cb;
>
> +       bool wide_bus_en;
> +
>         u32 max_pclk_khz;
>
>         u32 max_dp_lanes;
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index d7574e6..d413deb 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -399,6 +399,7 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display);
>  void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm_dp *dp_display);
>
>  void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor);
> +bool msm_dp_wide_bus_available(struct msm_dp *dp_display);
>
>  #else
>  static inline int __init msm_dp_register(void)
> @@ -449,6 +450,11 @@ static inline void msm_dp_debugfs_init(struct msm_dp *dp_display,
>  {
>  }
>
> +static inline bool msm_dp_wide_bus_available(struct msm_dp *dp_display)

const?

> +{
> +       return false;
> +}
> +
>  #endif
>
>  void __init msm_mdp_register(void);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>