Hi, Am Mittwoch, dem 14.06.2023 um 00:42 +0800 schrieb Sui Jingfeng: > Hi, Lucas > > > I love your patch, perhaps something to improve: > > The MLCG stand for "Module Level Clock Gating", > > without reading the commit message, I guess there may have people don't > know its meaning. > Yea, I expect people to read the commit message and not just the subject if they want to know what a patch does. The MLCG abbreviation is already well established within the driver code. > There are still more thing in this patch can only be understand relay on > guessing... :-) > That's unfortunately true. I don't know exactly what the pulse eater control bits mean either. I've taken them verbatim from the reset sequence in the Vivante kernel driver, which is also why I didn't try to assign some meaning by giving them a name, but keep them as BITs in the driver code. Regards, Lucas > > But drm/etnaviv drvier still works with this patch applied, so > > > On 2023/6/7 20:58, Lucas Stach wrote: > > Module level clock gating and the pulse eater might interfere with > > the GPU reset, as they both have the potential to stop the clock > > and thus reset propagation to parts of the GPU. > > > > Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > > Reviewed-by: Christian Gmeiner <cgmeiner@xxxxxxxxxx> > > > Tested-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx> > > > > --- > > I'm not aware of any cases where this would have been an issue, but > > it is what the downstream driver does and fundametally seems like > > the right thing to do. > > --- > > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > > index de8c9894967c..41aab1aa330b 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > > @@ -505,8 +505,19 @@ static int etnaviv_hw_reset(struct etnaviv_gpu *gpu) > > timeout = jiffies + msecs_to_jiffies(1000); > > > > while (time_is_after_jiffies(timeout)) { > > - /* enable clock */ > > unsigned int fscale = 1 << (6 - gpu->freq_scale); > > + u32 pulse_eater = 0x01590880; > > + > > + /* disable clock gating */ > > + gpu_write_power(gpu, VIVS_PM_POWER_CONTROLS, 0x0); > > + > > + /* disable pulse eater */ > > + pulse_eater |= BIT(17); > > + gpu_write_power(gpu, VIVS_PM_PULSE_EATER, pulse_eater); > > + pulse_eater |= BIT(0); > > + gpu_write_power(gpu, VIVS_PM_PULSE_EATER, pulse_eater); > > + > > + /* enable clock */ > > control = VIVS_HI_CLOCK_CONTROL_FSCALE_VAL(fscale); > > etnaviv_gpu_load_clock(gpu, control); > > >