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

[liviu.dudau at arm.com]

On Fri, May 17, 2019 at 10:37:01AM +0000, Wen He wrote:
> 
> 
> > -----Original Message-----
> > From: Robin Murphy [mailto:robin.murphy at arm.com]
> > Sent: 2019年5月16日 18:45
> > To: Wen He <wen.he_1 at nxp.com>; dri-devel at lists.freedesktop.org;
> > linux-kernel at vger.kernel.org; liviu.dudau at arm.com
> > Cc: Leo Li <leoyang.li at nxp.com>
> > Subject: Re: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required
> > pixel clock rate
> > 
> > 
> > On 16/05/2019 10:42, Wen He wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Robin Murphy [mailto:robin.murphy at arm.com]
> > >> Sent: 2019年5月16日 1:14
> > >> To: Wen He <wen.he_1 at nxp.com>; dri-devel at lists.freedesktop.org;
> > >> linux-kernel at vger.kernel.org; liviu.dudau at arm.com
> > >> Cc: Leo Li <leoyang.li at nxp.com>
> > >> Subject: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for
> > >> required pixel clock rate
> > >>
> > >> Caution: EXT Email
> > >>
> > >> On 15/05/2019 03:42, Wen He wrote:
> > >>> Disable checking for required pixel clock rate if ARCH_LAYERSCPAE is
> > >>> enable.
> > >>>
> > >>> Signed-off-by: Alison Wang <alison.wang at nxp.com>
> > >>> Signed-off-by: Wen He <wen.he_1 at nxp.com>
> > >>> ---
> > >>> 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.
> > >>>    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,
> > >>>
> > >>>        if (req_rate) {
> > >>>                rate = clk_round_rate(hwdev->pxlclk, req_rate);
> > >>> +#ifndef CONFIG_ARCH_LAYERSCAPE
> > >>
> > >> What about multiplatform builds? The kernel config doesn't tell you
> > >> what hardware you're actually running on.
> > >>
> > >
> > > Hi Robin,
> > >
> > > Thanks for your reply.
> > >
> > > In fact, Only one platform integrates this IP when
> > CONFIG_ARCH_LAYERSCAPE is set.
> > > Although this are not good ways, but I think it won't be a problem under
> > multiplatform builds.
> > 
> > My point is that ARCH_LAYERSCAPE is going to be enabled in distribution
> > kernels along with everything else, so you're effectively removing this check for
> > all other vendors' Mali-DP implementations as well, which is probably not OK.
> > 
> > Furthermore, if LS1028A really only supports 4 specific modes as the BSP
> > documentation I found claims, then surely you'd want a *more* specific check
> > here, rather than no check at all?
> 
> Hi Robin,
> 
> Thanks for your comments.
> 
> Yes, As you said, now LS1028A only supports 4 modes, and we use three clocks to support
> all four modes. In fact, this was really the point.
> 
> However, we can only enable one mode to meet the check statement in this place.
> 
> For example, If user has a 1080p monitor, we must be set the pixel fixed-clock to 148.5MHz. 
> if user want to choice 4k monitor, we also to be change the pixel fixed-clock to 594MHz in
> DT node. In reality, We have no way of knowing what kind of monitor the user wants. Right?

How does your DT know which monitor the user is going to plug in? Like I've
said, if you expose the mechanism you use to set the fixed-clock to a certain
value via the clk provider then you will be able to switch automatically to
that frequency without your patch.

Best regards,
Liviu

> 
> Moreover, user cannot to change screen resolution in this case, I don't think this place is
> reasonable. we need to supporting Ubuntu , Wayland and other embedded GU, so we need
> to switch the resolutions.
> 
> Maybe it's that most android device used, and android system always only need one
> resolution.
> 
> Best Regards,
> Wen
> 
> > 
> > Robin.

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯