NACK/Cmnt: [SRU][J][PATCH 1/1] drm/i915: Skip some timing checks on BXT/GLK DSI transcoders
Stefan Bader
stefan.bader at canonical.com
Wed Jan 31 08:50:51 UTC 2024
On 30.01.24 15:51, Dariusz Gadomski wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> BugLink: https://bugs.launchpad.net/bugs/2044131
>
> Apparently some BXT/GLK systems have DSI panels whose timings
> don't agree with the normal cpu transcoder hblank>=32 limitation.
> This is perhaps fine as there are no specific hblank/etc. limits
> listed for the BXT/GLK DSI transcoders.
>
> Move those checks out from the global intel_mode_valid() into
> into connector specific .mode_valid() hooks, skipping BXT/GLK
> DSI connectors. We'll leave the basic [hv]display/[hv]total
> checks in intel_mode_valid() as those seem like sensible upper
> limits regardless of the transcoder used.
>
> Cc: stable at vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9720
> Fixes: 8f4b1068e7fc ("drm/i915: Check some transcoder timing minimum limits")
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20231127145028.4899-1-ville.syrjala@linux.intel.com
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
> (cherry picked from commit e0ef2daa8ca8ce4dbc2fd0959e383b753a87fd7d)
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> (cherry picked from commit 20c2dbff342aec13bf93c2f6c951da198916a455)
> Signed-off-by: Dariusz Gadomski <dgadomski at ubuntu.com>
> ---
Rejected for the following reasons:
- Fix is upstream in v6.7-rc5 this should also be submitted for Mantic (6.5)
- Patch claims to be a cherry pick but the upstream patch does not apply
to Jammy (it does for Mantic though).
> drivers/gpu/drm/i915/display/icl_dsi.c | 7 +++++++
> drivers/gpu/drm/i915/display/intel_crt.c | 5 +++++
> drivers/gpu/drm/i915/display/intel_display.c | 10 ++++++++++
> drivers/gpu/drm/i915/display/intel_display.h | 3 +++
> drivers/gpu/drm/i915/display/intel_dp.c | 4 ++++
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 ++++
> drivers/gpu/drm/i915/display/intel_dvo.c | 16 +++++++++++-----
drivers/gpu/drm/i915/display/intel_dvo.c | 6 ++++++
> drivers/gpu/drm/i915/display/intel_hdmi.c | 4 ++++
> drivers/gpu/drm/i915/display/intel_lvds.c | 8 +++++++-
drivers/gpu/drm/i915/display/intel_lvds.c | 5 +++++
> drivers/gpu/drm/i915/display/intel_sdvo.c | 8 +++++++-
> drivers/gpu/drm/i915/display/intel_tv.c | 8 +++++++-
> drivers/gpu/drm/i915/display/vlv_dsi.c | 18 +++++++++++++++++-
> 12 files changed, 86 insertions(+), 9 deletions(-)
Please re-submit providing the original pick for Mantic and for Jammy
this would need to be a "(backported from..."
-Stefan
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 26dd5a2bd502..796e08de26b9 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1485,6 +1485,13 @@ static void gen11_dsi_post_disable(struct intel_atomic_state *state,
> static enum drm_mode_status gen11_dsi_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> {
> + struct drm_i915_private *i915 = to_i915(connector->dev);
> + enum drm_mode_status status;
> +
> + status = intel_cpu_transcoder_mode_valid(i915, mode);
> + if (status != MODE_OK)
> + return status;
> +
> /* FIXME: DSC? */
> return intel_dsi_mode_valid(connector, mode);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index 408f82b0dc7d..13ff3695a7b4 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -343,8 +343,13 @@ intel_crt_mode_valid(struct drm_connector *connector,
> struct drm_device *dev = connector->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> int max_dotclk = dev_priv->max_dotclk_freq;
> + enum drm_mode_status status;
> int max_clock;
>
> + status = intel_cpu_transcoder_mode_valid(dev_priv, mode);
> + if (status != MODE_OK)
> + return status;
> +
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> return MODE_NO_DBLESCAN;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 325040afa9c3..1662a7264734 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11778,6 +11778,16 @@ intel_mode_valid(struct drm_device *dev,
> mode->vtotal > vtotal_max)
> return MODE_V_ILLEGAL;
>
> + return MODE_OK;
> +}
> +
> +enum drm_mode_status intel_cpu_transcoder_mode_valid(struct drm_i915_private *dev_priv,
> + const struct drm_display_mode *mode)
> +{
> + /*
> + * Additional transcoder timing limits,
> + * excluding BXT/GLK DSI transcoders.
> + */
> if (DISPLAY_VER(dev_priv) >= 5) {
> if (mode->hdisplay < 64 ||
> mode->htotal - mode->hdisplay < 32)
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 8a59ecac174b..b3e6deac077c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -528,6 +528,9 @@ enum drm_mode_status
> intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> const struct drm_display_mode *mode,
> bool bigjoiner);
> +enum drm_mode_status
> +intel_cpu_transcoder_mode_valid(struct drm_i915_private *i915,
> + const struct drm_display_mode *mode);
> enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
> bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 2125cc7c9a9e..0ed5417aa8a5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -731,6 +731,10 @@ intel_dp_mode_valid(struct drm_connector *connector,
> enum drm_mode_status status;
> bool dsc = false, bigjoiner = false;
>
> + status = intel_cpu_transcoder_mode_valid(dev_priv, mode);
> + if (status != MODE_OK)
> + return status;
> +
> if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> return MODE_H_ILLEGAL;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 2a20487effcc..73ecba8b5459 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -697,6 +697,10 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
> return 0;
> }
>
> + *status = intel_cpu_transcoder_mode_valid(dev_priv, mode);
> + if (*status != MODE_OK)
> + return 0;
> +
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN) {
> *status = MODE_NO_DBLESCAN;
> return 0;
> diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c b/drivers/gpu/drm/i915/display/intel_dvo.c
> index 77419f8c05e9..c32784aeb1ca 100644
> --- a/drivers/gpu/drm/i915/display/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_dvo.c
> @@ -220,14 +220,20 @@ static void intel_enable_dvo(struct intel_atomic_state *state,
> }
>
> static enum drm_mode_status
> -intel_dvo_mode_valid(struct drm_connector *connector,
> +intel_dvo_mode_valid(struct drm_connector *_connector,
> struct drm_display_mode *mode)
> {
> - struct intel_dvo *intel_dvo = intel_attached_dvo(to_intel_connector(connector));
> - const struct drm_display_mode *fixed_mode =
> - to_intel_connector(connector)->panel.fixed_mode;
> - int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> + struct intel_connector *connector = to_intel_connector(_connector);
> + struct drm_i915_private *i915 = to_i915(connector->base.dev);
> + struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> + const struct drm_display_mode *fixed_mode = connector->panel.fixed_mode;
> + int max_dotclk = i915->max_dotclk_freq;
> int target_clock = mode->clock;
> + enum drm_mode_status status;
> +
> + status = intel_cpu_transcoder_mode_valid(i915, mode);
> + if (status != MODE_OK)
> + return status;
>
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> return MODE_NO_DBLESCAN;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 999c1821d750..bab105e6814e 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1962,6 +1962,10 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
> bool has_hdmi_sink = intel_has_hdmi_sink(hdmi, connector->state);
> bool ycbcr_420_only;
>
> + status = intel_cpu_transcoder_mode_valid(dev_priv, mode);
> + if (status != MODE_OK)
> + return status;
> +
> if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == DRM_MODE_FLAG_3D_FRAME_PACKING)
> clock *= 2;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> index 8f5741ebd58d..b17bdb71be04 100644
> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> @@ -387,8 +387,14 @@ intel_lvds_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> {
> struct intel_connector *intel_connector = to_intel_connector(connector);
> + struct drm_i915_private *i915 = to_i915(intel_connector->base.dev);
> struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
> - int max_pixclk = to_i915(connector->dev)->max_dotclk_freq;
> + int max_pixclk = i915->max_dotclk_freq;
> + enum drm_mode_status status;
> +
> + status = intel_cpu_transcoder_mode_valid(i915, mode);
> + if (status != MODE_OK)
> + return status;
>
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> return MODE_NO_DBLESCAN;
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index adb1693b1575..de2878f532e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -1863,13 +1863,19 @@ static enum drm_mode_status
> intel_sdvo_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> {
> + struct drm_i915_private *i915 = to_i915(connector->dev);
> struct intel_sdvo *intel_sdvo = intel_attached_sdvo(to_intel_connector(connector));
> struct intel_sdvo_connector *intel_sdvo_connector =
> to_intel_sdvo_connector(connector);
> - int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> + int max_dotclk = i915->max_dotclk_freq;
> bool has_hdmi_sink = intel_has_hdmi_sink(intel_sdvo, connector->state);
> + enum drm_mode_status status;
> int clock = mode->clock;
>
> + status = intel_cpu_transcoder_mode_valid(i915, mode);
> + if (status != MODE_OK)
> + return status;
> +
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> return MODE_NO_DBLESCAN;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
> index d02f09f7e750..0ca392d6ff79 100644
> --- a/drivers/gpu/drm/i915/display/intel_tv.c
> +++ b/drivers/gpu/drm/i915/display/intel_tv.c
> @@ -955,8 +955,14 @@ static enum drm_mode_status
> intel_tv_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> {
> + struct drm_i915_private *i915 = to_i915(connector->dev);
> const struct tv_mode *tv_mode = intel_tv_mode_find(connector->state);
> - int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
> + int max_dotclk = i915->max_dotclk_freq;
> + enum drm_mode_status status;
> +
> + status = intel_cpu_transcoder_mode_valid(i915, mode);
> + if (status != MODE_OK)
> + return status;
>
> if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
> return MODE_NO_DBLESCAN;
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index b27738df447d..e25129ab915d 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -1613,9 +1613,25 @@ static const struct drm_encoder_funcs intel_dsi_funcs = {
> .destroy = intel_dsi_encoder_destroy,
> };
>
> +static enum drm_mode_status vlv_dsi_mode_valid(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> +{
> + struct drm_i915_private *i915 = to_i915(connector->dev);
> +
> + if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) {
> + enum drm_mode_status status;
> +
> + status = intel_cpu_transcoder_mode_valid(i915, mode);
> + if (status != MODE_OK)
> + return status;
> + }
> +
> + return intel_dsi_mode_valid(connector, mode);
> +}
> +
> static const struct drm_connector_helper_funcs intel_dsi_connector_helper_funcs = {
> .get_modes = intel_dsi_get_modes,
> - .mode_valid = intel_dsi_mode_valid,
> + .mode_valid = vlv_dsi_mode_valid,
> .atomic_check = intel_digital_connector_atomic_check,
> };
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 48643 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240131/ba775337/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240131/ba775337/attachment-0001.sig>
More information about the kernel-team
mailing list