| Message ID | 20230322091900.1982453-8-stanislaw.gruszka@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | acell/ivpu: Fixes for 6.3 | expand |
On 3/22/2023 3:19 AM, Stanislaw Gruszka wrote: > VPU cpu clock frequency depends on the workpoint configuration > that was granted by the punit. Previously driver was passing > incorrect frequency to the VPU firmware. This looks like past tense. I believe the preference is to use the present tense for commit text. Something like "the driver calculates the wrong frequency because it ignores the workpoint config and this causes X. Fix this by using the workpoint config in the freq calculations". Should this have a fixes tag? Sounds like this addresses a bug. > Co-developed-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com> > Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > --- > drivers/accel/ivpu/ivpu_hw_mtl.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/accel/ivpu/ivpu_hw_mtl.c b/drivers/accel/ivpu/ivpu_hw_mtl.c > index 98c8a4aa25f0..382ec127be8e 100644 > --- a/drivers/accel/ivpu/ivpu_hw_mtl.c > +++ b/drivers/accel/ivpu/ivpu_hw_mtl.c > @@ -29,7 +29,6 @@ > > #define PLL_REF_CLK_FREQ (50 * 1000000) > #define PLL_SIMULATION_FREQ (10 * 1000000) > -#define PLL_RATIO_TO_FREQ(x) ((x) * PLL_REF_CLK_FREQ) > #define PLL_DEFAULT_EPP_VALUE 0x80 > > #define TIM_SAFE_ENABLE 0xf1d0dead > @@ -789,6 +788,19 @@ static void ivpu_hw_mtl_wdt_disable(struct ivpu_device *vdev) > REGV_WR32(MTL_VPU_CPU_SS_TIM_GEN_CONFIG, val); > } > > +static u32 ivpu_hw_mtl_pll_to_freq(u32 ratio, u32 config) > +{ > + u32 pll_clock = PLL_REF_CLK_FREQ * ratio; > + u32 cpu_clock; > + > + if ((config & 0xff) == PLL_RATIO_4_3) > + cpu_clock = pll_clock * 2 / 4; > + else > + cpu_clock = pll_clock * 2 / 5; > + > + return cpu_clock; > +} > + > /* Register indirect accesses */ > static u32 ivpu_hw_mtl_reg_pll_freq_get(struct ivpu_device *vdev) > { > @@ -800,7 +812,7 @@ static u32 ivpu_hw_mtl_reg_pll_freq_get(struct ivpu_device *vdev) > if (!ivpu_is_silicon(vdev)) > return PLL_SIMULATION_FREQ; > > - return PLL_RATIO_TO_FREQ(pll_curr_ratio); > + return ivpu_hw_mtl_pll_to_freq(pll_curr_ratio, vdev->hw->config); > } > > static u32 ivpu_hw_mtl_reg_telemetry_offset_get(struct ivpu_device *vdev)
On Wed, Mar 22, 2023 at 08:52:56AM -0600, Jeffrey Hugo wrote: > On 3/22/2023 3:19 AM, Stanislaw Gruszka wrote: > > VPU cpu clock frequency depends on the workpoint configuration > > that was granted by the punit. Previously driver was passing > > incorrect frequency to the VPU firmware. > > This looks like past tense. I believe the preference is to use the present > tense for commit text. Something like "the driver calculates the wrong > frequency because it ignores the workpoint config and this causes X. Fix > this by using the workpoint config in the freq calculations". Will do. > Should this have a fixes tag? Sounds like this addresses a bug. Not sure how this is done, but seems all my previous patches for ivpu have Fixes tag in commit message, even if I did not post the tag in the patches. Seems to be a feature of drm git tooling and can be easily done by committer ? Regards Stanislaw
On Wed, Mar 22, 2023 at 05:28:48PM +0100, Stanislaw Gruszka wrote: > On Wed, Mar 22, 2023 at 08:52:56AM -0600, Jeffrey Hugo wrote: > > On 3/22/2023 3:19 AM, Stanislaw Gruszka wrote: > > > VPU cpu clock frequency depends on the workpoint configuration > > > that was granted by the punit. Previously driver was passing > > > incorrect frequency to the VPU firmware. > > > > This looks like past tense. I believe the preference is to use the present > > tense for commit text. Something like "the driver calculates the wrong > > frequency because it ignores the workpoint config and this causes X. Fix > > this by using the workpoint config in the freq calculations". > > Will do. > > > Should this have a fixes tag? Sounds like this addresses a bug. > > Not sure how this is done, but seems all my previous patches for ivpu > have Fixes tag in commit message, even if I did not post the tag in > the patches. Seems to be a feature of drm git tooling and can be > easily done by committer ? Tags were added by Jacek, I'll add them to the patchset then. Regards Stanislaw
diff --git a/drivers/accel/ivpu/ivpu_hw_mtl.c b/drivers/accel/ivpu/ivpu_hw_mtl.c index 98c8a4aa25f0..382ec127be8e 100644 --- a/drivers/accel/ivpu/ivpu_hw_mtl.c +++ b/drivers/accel/ivpu/ivpu_hw_mtl.c @@ -29,7 +29,6 @@ #define PLL_REF_CLK_FREQ (50 * 1000000) #define PLL_SIMULATION_FREQ (10 * 1000000) -#define PLL_RATIO_TO_FREQ(x) ((x) * PLL_REF_CLK_FREQ) #define PLL_DEFAULT_EPP_VALUE 0x80 #define TIM_SAFE_ENABLE 0xf1d0dead @@ -789,6 +788,19 @@ static void ivpu_hw_mtl_wdt_disable(struct ivpu_device *vdev) REGV_WR32(MTL_VPU_CPU_SS_TIM_GEN_CONFIG, val); } +static u32 ivpu_hw_mtl_pll_to_freq(u32 ratio, u32 config) +{ + u32 pll_clock = PLL_REF_CLK_FREQ * ratio; + u32 cpu_clock; + + if ((config & 0xff) == PLL_RATIO_4_3) + cpu_clock = pll_clock * 2 / 4; + else + cpu_clock = pll_clock * 2 / 5; + + return cpu_clock; +} + /* Register indirect accesses */ static u32 ivpu_hw_mtl_reg_pll_freq_get(struct ivpu_device *vdev) { @@ -800,7 +812,7 @@ static u32 ivpu_hw_mtl_reg_pll_freq_get(struct ivpu_device *vdev) if (!ivpu_is_silicon(vdev)) return PLL_SIMULATION_FREQ; - return PLL_RATIO_TO_FREQ(pll_curr_ratio); + return ivpu_hw_mtl_pll_to_freq(pll_curr_ratio, vdev->hw->config); } static u32 ivpu_hw_mtl_reg_telemetry_offset_get(struct ivpu_device *vdev)