RE: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required pixel clock rate

[Wen He]

> -----Original Message-----
> From: liviu.dudau@xxxxxxx [mailto:liviu.dudau@xxxxxxx]
> Sent: 2019å5æ15æ 23:46
> To: Wen He <wen.he_1@xxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Leo Li
> <leoyang.li@xxxxxxx>
> Subject: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required pixel
> clock rate
> 
> 
> Hi Wen,

Hi Liviu,

> 
> On Wed, May 15, 2019 at 02:42:08AM +0000, Wen He wrote:
> > Disable checking for required pixel clock rate if ARCH_LAYERSCPAE is
> > enable.
> >
> > Signed-off-by: Alison Wang <alison.wang@xxxxxxx>
> > Signed-off-by: Wen He <wen.he_1@xxxxxxx>
> > ---
> > change in description:
> >       - This check that only supported one pixel clock required clock rate
> >       compare with dts node value. but we have supports 4 pixel clock
> >       for ls1028a board.
> 
> So, your DT says your pixel clock provider is a fixed clock? If you support more
> than one rate, you should instead use a real provider for it. How do you
> support the 4 pixel clocks?
> 
 
Yes , the DT node only can provided one pixel clock by using a fixed clock.
But we Display Port controller support 4 or more resolutions, each of which
requires a set of pixel clocks to drive, and we hope they can switch any resolution
we want by some program every times.

For example, if we set that fixed pixel clock is 27000000 (27Mhz), but user hope can see
a group 1080p resolution penguins during startup , and hope playing a 4k video once
system boot up done. 
Btw, In our board, the 1080p resolution is driven by a 148.5Mhz pixel clock, 4k is driven
by a 594Mhz. 27Mhz only can drive 480p resolution.

To meet the above user requirements, I was to setup following steps,
1. Add the "video=1920x1080-32@60" to bootargs command line [specify penguins size]
2. Play a 4K video with 4k resolution when system boot up done.

> Also, not sure what the paragraph above is meant to be. Should it be part of
> the commit message?
> 

These comments just want to let you know.

> Best regards,
> Liviu
> 
> 
> >  drivers/gpu/drm/arm/malidp_crtc.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> > b/drivers/gpu/drm/arm/malidp_crtc.c
> > index 56aad288666e..bb79223d9981 100644
> > --- a/drivers/gpu/drm/arm/malidp_crtc.c
> > +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> > @@ -36,11 +36,13 @@ static enum drm_mode_status
> > malidp_crtc_mode_valid(struct drm_crtc *crtc,
> >

According to our pixel configuration above,
Now the variable req_rate value is 148500000 or 59400000, another variable rate value is
27000000, so we will get a warning and display will cannot works well. 

We're not sure which resolution are user want, and we also can't just offered one resolution
to user. so I remove this check on our board, maybe it's not good change.

I want to know do you have other good suggestion? Thanks.

Best Regards,
Wen

> >       if (req_rate) {
> >               rate = clk_round_rate(hwdev->pxlclk, req_rate);
> > +#ifndef CONFIG_ARCH_LAYERSCAPE
> >               if (rate != req_rate) {
> >                       DRM_DEBUG_DRIVER("pxlclk doesn't support %ld
> Hz\n",
> >                                        req_rate);
> >                       return MODE_NOCLOCK;
> >               }
> > +#endif
> >       }
> >
> >       return MODE_OK;
> > --
> > 2.17.1
> >
> 
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     Â\_(ã)_/Â