Re: [d-kernel] [PATCH 1/1] Revert "drm/edid: Fix csync detailed mode parsing"

[Vitaly Chikunov <vt@altlinux.org>]

Vasiliy,

On Wed, Dec 06, 2023 at 04:33:01PM +0300, kovalev@altlinux.org wrote:
> From: Vasiliy Kovalev <kovalev@altlinux.org>
> 
> This reverts commit 5a46dc8e4a064769e916d87bf9bccae75afc7289.
> 
> Commit 50b6f2c8297793f7f3315623db78dcff85158e96 upstream.
> 
> Commit 5a46dc8e4a0647 ("drm/edid: Fix csync detailed mode parsing") fixed
> EDID detailed mode sync parsing. Unfortunately, there are quite a few
> displays out there that have bogus (zero) sync field that are broken by
> the change. Zero means analog composite sync, which is not right for
> digital displays, and the modes get rejected. Regardless, it used to
> work, and it needs to continue to work. Revert the change.
> 
> Rejecting modes with analog composite sync was the part that fixed the
> gitlab issue 8146 [1]. We'll need to get back to the drawing board with
> that.

1. Если этот патч отослан в апстрм[1], то зачем он здесь? Видимо, стоит
подождать решения апсрима и прихода через апстрим или переделки патча
если апстрим найдет проблему. У нас не ядра для тестирования изменений
перед отсылкой в апстрим, а наоборот мы получаем коммиты из апстрима
которые уже прошли весь цикл рецензирования и тестирования.

Мы принимаем все патчи из апсрима, так что без серьезных причин не стоит
их дублировать здесь. У нас нет ресурсов делать свой franken-браня ядра
конфликтующий с апстримом.

  [1] https://lore.kernel.org/all/20231206084946.111835-2-kovalev@altlinux.org/

2. Например, точно так же патчи не шлются только или напрямую в stable,
а шлются в mainline, а оттуда перекладываются в stable.

   https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
   "There are three options to submit a change to -stable trees".

3. Например, так же происходит с Android Common Kernels

   https://android.googlesource.com/kernel/common/
   "BEST: Make all of your changes to upstream Linux."


> 
> [1] https://gitlab.freedesktop.org/drm/intel/-/issues/8146
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8789
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8930
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9044
> Fixes: 5a46dc8e4a0647 ("drm/edid: Fix csync detailed mode parsing")
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v6.4+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20230815101907.2900768-1-jani.nikula@intel.com
> Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
> ---
>  drivers/gpu/drm/drm_edid.c | 26 +++++++-------------------
>  include/drm/drm_edid.h     | 12 +++---------
>  2 files changed, 10 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index a26c6c57a627c0..5ed77e3361fd72 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3307,6 +3307,9 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
>  		DRM_DEBUG_KMS("stereo mode not supported\n");
>  		return NULL;
>  	}
> +	if (!(pt->misc & DRM_EDID_PT_SEPARATE_SYNC)) {
> +		DRM_DEBUG_KMS("composite sync not supported\n");
> +	}
>  
>  	/* it is incorrect if hsync/vsync width is zero */
>  	if (!hsync_pulse_width || !vsync_pulse_width) {
> @@ -3353,25 +3356,10 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
>  	if (quirks & EDID_QUIRK_DETAILED_SYNC_PP) {
>  		mode->flags |= DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC;
>  	} else {
> -		switch (pt->misc & DRM_EDID_PT_SYNC_MASK) {
> -		case DRM_EDID_PT_ANALOG_CSYNC:
> -		case DRM_EDID_PT_BIPOLAR_ANALOG_CSYNC:
> -			drm_dbg_kms(dev, "Analog composite sync!\n");
> -			mode->flags |= DRM_MODE_FLAG_CSYNC | DRM_MODE_FLAG_NCSYNC;
> -			break;
> -		case DRM_EDID_PT_DIGITAL_CSYNC:
> -			drm_dbg_kms(dev, "Digital composite sync!\n");
> -			mode->flags |= DRM_MODE_FLAG_CSYNC;
> -			mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?
> -				DRM_MODE_FLAG_PCSYNC : DRM_MODE_FLAG_NCSYNC;
> -			break;
> -		case DRM_EDID_PT_DIGITAL_SEPARATE_SYNC:
> -			mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?
> -				DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
> -			mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?
> -				DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
> -			break;
> -		}
> +		mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?
> +			DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
> +		mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?
> +			DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
>  	}
>  
>  set_size:
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 008d2ed39822ec..1ed61e2b30a41c 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -61,15 +61,9 @@ struct std_timing {
>  	u8 vfreq_aspect;
>  } __attribute__((packed));
>  
> -#define DRM_EDID_PT_SYNC_MASK              (3 << 3)
> -# define DRM_EDID_PT_ANALOG_CSYNC          (0 << 3)
> -# define DRM_EDID_PT_BIPOLAR_ANALOG_CSYNC  (1 << 3)
> -# define DRM_EDID_PT_DIGITAL_CSYNC         (2 << 3)
> -#  define DRM_EDID_PT_CSYNC_ON_RGB         (1 << 1) /* analog csync only */
> -#  define DRM_EDID_PT_CSYNC_SERRATE        (1 << 2)
> -# define DRM_EDID_PT_DIGITAL_SEPARATE_SYNC (3 << 3)
> -#  define DRM_EDID_PT_HSYNC_POSITIVE       (1 << 1) /* also digital csync */
> -#  define DRM_EDID_PT_VSYNC_POSITIVE       (1 << 2)
> +#define DRM_EDID_PT_HSYNC_POSITIVE (1 << 1)
> +#define DRM_EDID_PT_VSYNC_POSITIVE (1 << 2)
> +#define DRM_EDID_PT_SEPARATE_SYNC  (3 << 3)
>  #define DRM_EDID_PT_STEREO         (1 << 5)
>  #define DRM_EDID_PT_INTERLACED     (1 << 7)
>  
> -- 
> 2.33.8
> 
> _______________________________________________
> devel-kernel mailing list
> devel-kernel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel-kernel