[PATCH 05/14] drm/msm/dp: fold dp_power into dp_ctrl module

[Dmitry Baryshkov]

On Sat, 30 Dec 2023 at 02:12, Konrad Dybcio <konrad.dybcio at linaro.org> wrote:
>
> On 29.12.2023 23:56, Dmitry Baryshkov wrote:
> > The dp_power submodule is limited to handling the clocks only following
> > previous cleanups. Fold it into the dp_ctrl submodule, removing one
> > unnecessary level of indirection.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> > ---
> [...]
>
> >
> > -     /* set dongle to D3 (power off) mode */
> > +     /* set dongle to D3 (ctrl off) mode */
> :thinking emoji:
>
> [...]
>
> > +int dp_ctrl_clk_enable(struct dp_ctrl *dp_ctrl,
> > +             enum dp_pm_type pm_type, bool enable)
> > +{
> > +     int rc = 0;
> > +     struct dp_ctrl_private *ctrl;
> > +     struct dss_module_power *mp;
> I haven't checked out the rest of the series yet, but cleaning
> this up with rc->ret and reverse-Christmas-tree (merry late
> Christmas!) would be nice
>
> > +
> > +     ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
> > +
> > +     if (pm_type != DP_CORE_PM && pm_type != DP_CTRL_PM &&
> > +                     pm_type != DP_STREAM_PM) {
> > +             DRM_ERROR("unsupported ctrl module: %s\n",
> > +                             dp_parser_pm_name(pm_type));
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (enable) {
> > +             if (pm_type == DP_CORE_PM && ctrl->core_clks_on) {
> again, haven't yet looked through all the patches but this screams
> "if (enable && pm_type == DP_CORE_PM)"

I'd prefer to keep it as is, it follows the existing code and it is
also more idiomatic: we perform several checks if we were told to
enable clocks.

-- 
With best wishes
Dmitry