On Fri, May 17, 2019 at 10:37:01AM +0000, Wen He wrote:
>
>
> > -----Original Message-----
> > From: Robin Murphy [mailto:robin.mur...@arm.com]
> > Sent: 2019年5月16日 18:45
> > To: Wen He <wen.h...@nxp.com>; dri-devel@lists.freedesktop.org;
> > linux-ker...@vger.kernel.org; liviu.du...@arm.com
> > Cc: Leo Li <leoyang...@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.mur...@arm.com]
> > >> Sent: 2019年5月16日 1:14
> > >> To: Wen He <wen.h...@nxp.com>; dri-devel@lists.freedesktop.org;
> > >> linux-ker...@vger.kernel.org; liviu.du...@arm.com
> > >> Cc: Leo Li <leoyang...@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.w...@nxp.com>
> > >>> Signed-off-by: Wen He <wen.h...@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! /
---------------
¯\_(ツ)_/¯
