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

[Wen He]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@xxxxxxx]
> Sent: 2019å5æ16æ 18:45
> To: Wen He <wen.he_1@xxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; liviu.dudau@xxxxxxx
> Cc: Leo Li <leoyang.li@xxxxxxx>
> 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@xxxxxxx]
> >> Sent: 2019å5æ16æ 1:14
> >> To: Wen He <wen.he_1@xxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> >> linux-kernel@xxxxxxxxxxxxxxx; liviu.dudau@xxxxxxx
> >> Cc: Leo Li <leoyang.li@xxxxxxx>
> >> 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@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.
> >>>    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?

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.