On Fri, 17 Jan 2020, Lee Shawn C <[email protected]> wrote:
> While mode setting, driver would calculate mode rate based on
> resolution and bpp. And choose the best bpp that did not exceed
> DP bandwidtd.
>
> But LSPCON had more restriction due to it convert DP to HDMI.
> Driver should respect HDMI's bandwidth limitation if LSPCON
> was active. This change would ignore the bpp when its required
> output bandwidth already over HDMI 2.0 or 1.4 spec.
>
> Cc: Imre Deak <[email protected]>
> Cc: Ville Syrjälä <[email protected]>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Cooper Chiou <[email protected]>
> Cc: Sam McNally <[email protected]>
> Signed-off-by: Lee Shawn C <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     | 45 +++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_lspcon.c |  5 +++
>  drivers/gpu/drm/i915/display/intel_lspcon.h |  1 +
>  3 files changed, 51 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index c7424e2a04a3..c27d3e7ac219 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1976,6 +1976,47 @@ static int intel_dp_output_bpp(const struct 
> intel_crtc_state *crtc_state, int bp
>       return bpp;
>  }
>  
> +static bool
> +intel_dp_lspcon_exceed_bandwidth_limitation(struct intel_dp *intel_dp,
> +                                         struct intel_crtc_state 
> *pipe_config,
> +                                         int bpp)
> +{
> +     struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
> +     struct intel_connector *connector = intel_dp->attached_connector;
> +     const struct drm_display_info *info = &connector->base.display_info;
> +     enum drm_lspcon_mode lspcon_current_mode = lspcon_get_mode(lspcon);
> +     const int pcon_mode_max_tmds_clock = 600000;
> +     const int ls_mode_max_tmds_clock   = 340000;
> +     int mode_rate, max_tmds_clock = pcon_mode_max_tmds_clock;
> +
> +     if (lspcon->active) {
> +             switch (bpp) {
> +             case 36:
> +                     mode_rate = pipe_config->hw.adjusted_mode.crtc_clock * 
> 3 / 2;
> +                     break;
> +             case 30:
> +                     mode_rate = pipe_config->hw.adjusted_mode.crtc_clock * 
> 5 / 4;
> +                     break;
> +             case 24:
> +             default:
> +                     mode_rate = pipe_config->hw.adjusted_mode.crtc_clock;
> +                     break;
> +             }
> +
> +             if (lspcon_current_mode == DRM_LSPCON_MODE_LS)
> +                     max_tmds_clock = ls_mode_max_tmds_clock;
> +
> +             if (info->max_tmds_clock)
> +                     max_tmds_clock = min(max_tmds_clock,
> +                                          info->max_tmds_clock);
> +
> +             if (mode_rate > max_tmds_clock)
> +                     return true;
> +     }
> +
> +     return false;
> +}
> +

Instead of this, please add a simple intel_lspcon.c function:

        int lspcon_max_rate(struct intel_lspcon *lspcon);

that returns the max rate. Everything else can then be done in
intel_dp.c around this. The function gets simplified enough that you can
throw out the const ints and use the values directly.

>  /* Optimize link config in order: max bpp, min clock, min lanes */
>  static int
>  intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
> @@ -1989,6 +2030,10 @@ intel_dp_compute_link_config_wide(struct intel_dp 
> *intel_dp,
>       for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
>               int output_bpp = intel_dp_output_bpp(pipe_config, bpp);
>  
> +             /* Bypass th bpp if require bandwidth over HDMI spec when 
> LSPCON active */
> +             if (intel_dp_lspcon_exceed_bandwidth_limitation(intel_dp, 
> pipe_config, output_bpp))
> +                     continue;
> +

The placing sticks out like a sore thumb. I think we need to filter out
the modes already in intel_dp_mode_valid(). This isn't all that
different from intel_dp_downstream_max_dotclock() is it?

>               mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
>                                                  output_bpp);

But I guess since the mode valid limit is for 8 bpc, you'll also need a
check here? Maybe Ville has better ideas.

Here it would be something like:

        if (lspcon->active && mode_rate > lspcon_max_rate(lscon))
                continue;

BR,
Jani.

>  
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c 
> b/drivers/gpu/drm/i915/display/intel_lspcon.c
> index d807c5648c87..6952c5028fdf 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
> @@ -550,6 +550,11 @@ void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon)
>       lspcon_wait_mode(lspcon, DRM_LSPCON_MODE_PCON);
>  }
>  
> +int lspcon_get_mode(struct intel_lspcon *lspcon)
> +{
> +     return lspcon_get_current_mode(lspcon);
> +}
> +
>  bool lspcon_init(struct intel_digital_port *intel_dig_port)
>  {
>       struct intel_dp *dp = &intel_dig_port->dp;
> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.h 
> b/drivers/gpu/drm/i915/display/intel_lspcon.h
> index 37cfddf8a9c5..5ce9daef9708 100644
> --- a/drivers/gpu/drm/i915/display/intel_lspcon.h
> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.h
> @@ -18,6 +18,7 @@ struct intel_lspcon;
>  bool lspcon_init(struct intel_digital_port *intel_dig_port);
>  void lspcon_resume(struct intel_lspcon *lspcon);
>  void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
> +int lspcon_get_mode(struct intel_lspcon *lspcon);
>  void lspcon_write_infoframe(struct intel_encoder *encoder,
>                           const struct intel_crtc_state *crtc_state,
>                           unsigned int type,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to