[RFC][PATCH v2] drm: kirin: Add mode_valid logic to avoid mode clocks we can't generate

[Jose Abreu]

Hi John,


On 18-07-2017 05:22, John Stultz wrote:
> Currently the hikey dsi logic cannot generate accurate byte
> clocks values for all pixel clock values. Thus if a mode clock
> is selected that cannot match the calculated byte clock, the
> device will boot with a blank screen.
>
> This patch uses the new mode_valid callback (many thanks to
> Jose Abreu for upstreaming it!) to ensure we don't select
> modes we cannot generate.
>
> NOTE: Stylistically I suspect there are better ways to do what
> I'm trying to do here. The encoder -> crtc bit is terrible, and
> getting the crtc adjusted mode from the encoder logic feels
> less then ideal. So feedback would be greatly appreciated!
>
> Cc: Daniel Vetter <daniel.vetter at intel.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Sean Paul <seanpaul at chromium.org>
> Cc: David Airlie <airlied at linux.ie>
> Cc: Rob Clark <robdclark at gmail.com>
> Cc: Xinliang Liu <xinliang.liu at linaro.org>
> Cc: Xinliang Liu <z.liuxinliang at hisilicon.com>
> Cc: Rongrong Zou <zourongrong at gmail.com>
> Cc: Xinwei Kong <kong.kongxinwei at hisilicon.com>
> Cc: Chen Feng <puck.chen at hisilicon.com>
> Cc: Jose Abreu <Jose.Abreu at synopsys.com>
> Cc: Archit Taneja <architt at codeaurora.org>
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: John Stultz <john.stultz at linaro.org>
> ---
> v2: Reworked to calculate if modeclock matches the phy's byteclock,
>     rather then using a whitelist of known modes.

Something like Daniel suggested would be simpler maybe:

encoder_mode_valid_aux()
{
    ...
}

dsi_encoder_mode_valid(...)
{
    drm_for_each_crtc(crtc, dev) {
        crtc->mode_fixup(crtc, mode, adjusted_mode);
        ret = encoder_mode_valid_aux(adjusted_mode);
        if (ret != MODE_OK)
            return ret;
    }
}

BTW, I think at commit stage you have encoder->crtc populated,
not sure though. But at probbing stage it will not be bound. And
also if you have more than one crtc this may be wrong. (How will
you know which crtc will be bound to the encoder so that you can
get the right clock?).

Best regards,
Jose Miguel Abreu