> -----Original Message-----
> From: Jani Nikula <jani.nik...@linux.intel.com>
> Sent: Wednesday, 3 September 2025 15.41
> To: Kahola, Mika <mika.kah...@intel.com>; intel-gfx@lists.freedesktop.org;
> intel...@lists.freedesktop.org
> Subject: RE: [PATCH] drm/i915/display: Initialize phy and ch variables
>
> On Wed, 03 Sep 2025, "Kahola, Mika" <mika.kah...@intel.com> wrote:
> > Ok good to know. Let's just ignore this patch.
>
> That said, all places that use bxt_port_to_phy_channel(),
> vlv_dig_port_to_channel(), vlv_dig_port_to_phy(),
> vlv_pipe_to_channel(), or vlv_pipe_to_phy() need *both* the phy and channel.
>
> So I'm thinking you could add
>
> struct {
> enum dpio_phy phy;
> enum dpio_channel channel;
> } dpio_phy_channel;
>
> and convert the above functions to always return that struct. It's small
> enough to return as a whole. Something like:
>
> struct dpio_phy_channel phy_channel;
>
> phy_channel = bxt_port_to_phy_channel(display, port);
>
> and
>
> phy_channel = vlv_dig_port_to_phy_channel(dig_port);
>
> and
>
> phy_channel = vlv_pipe_to_phy_channel(pipe);
>
> Then you can also do s/phy/phy_channel.phy/ and s/ch/phy_channel.ch/ as
> cleanup (if it actually cleans things up, maybe not).
>
> Bonus points if you can come up with a better name than "phy_channel". ;)
>
> With that, the interface gets reduced for vlv, and it's obvious to static
> analyzers the stuff gets initialized. Win-win?
Thanks Jani! This is something I could implement rather than simply checking
the return values of the functions.
-Mika-
>
>
> BR,
> Jani.
>
>
> >
> > Thanks!
> >
> > -Mika-
> >
> >> -----Original Message-----
> >> From: Jani Nikula <jani.nik...@linux.intel.com>
> >> Sent: Wednesday, 3 September 2025 15.19
> >> To: Kahola, Mika <mika.kah...@intel.com>;
> >> intel-gfx@lists.freedesktop.org; intel...@lists.freedesktop.org
> >> Cc: Kahola, Mika <mika.kah...@intel.com>
> >> Subject: Re: [PATCH] drm/i915/display: Initialize phy and ch
> >> variables
> >>
> >> On Wed, 03 Sep 2025, Mika Kahola <mika.kah...@intel.com> wrote:
> >> > phy and ch variables are potentially used uninitialized.
> >>
> >> They're not, bxt_port_to_phy_channel() initializes them in all code paths.
> >>
> >> > To make absolutely sure that these variables are not used
> >> > uninitialized let's initialize these with known values as
> >> > DPIO_PHY0 and DPIO_CH0, respectively.
> >> >
> >> > Signed-off-by: Mika Kahola <mika.kah...@intel.com>
> >> > ---
> >> > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 4 ++--
> >> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> >> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> >> > index 8ea96cc524a1..45b67a3716e9 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> >> > @@ -2174,8 +2174,8 @@ static bool bxt_ddi_pll_get_hw_state(struct
> >> > intel_display *display,
> >> > struct bxt_dpll_hw_state *hw_state = &dpll_hw_state->bxt;
> >> > enum port port = (enum port)pll->info->id; /* 1:1 port->PLL
> >> > mapping */
> >> > intel_wakeref_t wakeref;
> >> > - enum dpio_phy phy;
> >> > - enum dpio_channel ch;
> >> > + enum dpio_phy phy = DPIO_PHY0;
> >> > + enum dpio_channel ch = DPIO_CH0;
> >> > u32 val;
> >> > bool ret;
> >>
> >> --
> >> Jani Nikula, Intel
>
> --
> Jani Nikula, Intel
