Re: [PATCH 01/11] drm: Add HDMI 2.0 VIC support for AVI info-frames

2017-04-19 Thread Sharma, Shashank

Regards

Shashank


On 4/10/2017 3:17 PM, Andrzej Hajda wrote:

On 07.04.2017 18:39, Shashank Sharma wrote:

HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
For any other mode, the VIC filed in AVI infoframes should be 0.
HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
extended to (VIC 1-107).

This patch adds a bool input variable, which indicates if the connected
sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
HDMI 2.0 VIC to a HDMI 1.4 sink.

This patch touches all drm drivers, who are callers of this function
drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
no change in current behavior, is_hdmi2 is kept as false.

In case of I915 driver, this patch checks the connector->display_info
to check if the connected display is HDMI 2.0.

Cc: Ville Syrjala 
Cc: Jose Abreu 
Cc: Andrzej Hajda 
Cc: Alex Deucher 
Cc: Daniel Vetter 

PS: This patch touches a few lines in few files, which were
already above 80 char, so checkpatch gives 80 char warning again.
- gpu/drm/omapdrm/omap_encoder.c
- gpu/drm/i915/intel_sdvo.c

Signed-off-by: Shashank Sharma 

Finally I have chances to look at the specs and I am not sure if this
solution fully reflects the specs and is scalable. According to specs
VIC set to 0 in AVIF means "Video Format not documented in CTA-861", ie
it is described by vendor specific data block and vendor specific
infoframe, maybe something else(???).

It says "Video Format not documented in CEA-861 (means this version)"

I suppose ideally during EDID read
there should be recorded info for every mode if it was defined by vendor
extension, or not. This info could be later used by drivers to configure
AVIF and VSIF accordingly.
I don't think this has to do anything with it, we are simply checking if 
the monitor is HDMI 2.0 capable,
coz if it is, it would be complaint to CEA-861-F and it would understand 
the VIC>64. If not && VIC>64, we will send
VIC as 0, coz this VIC is defined in CEA-861-F spec. So I don't see the 
confusion.


You are welcome to suggest, how can we make it more scalable, or improve 
on it. We can always send revised patch set :-)


- Shashank


Anyway as a short-term initial solution it could work.
So:
Reviewed-by: Andrzej Hajda 

  --
Regards
Andrzej


---
  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  2 +-
  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  2 +-
  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  2 +-
  drivers/gpu/drm/bridge/analogix-anx78xx.c |  3 ++-
  drivers/gpu/drm/bridge/sii902x.c  |  2 +-
  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
  drivers/gpu/drm/drm_edid.c| 12 +++-
  drivers/gpu/drm/exynos/exynos_hdmi.c  |  2 +-
  drivers/gpu/drm/i2c/tda998x_drv.c |  2 +-
  drivers/gpu/drm/i915/intel_hdmi.c |  5 -
  drivers/gpu/drm/i915/intel_sdvo.c |  3 ++-
  drivers/gpu/drm/mediatek/mtk_hdmi.c   |  2 +-
  drivers/gpu/drm/omapdrm/omap_encoder.c|  3 ++-
  drivers/gpu/drm/radeon/radeon_audio.c |  2 +-
  drivers/gpu/drm/rockchip/inno_hdmi.c  |  2 +-
  drivers/gpu/drm/sti/sti_hdmi.c|  2 +-
  drivers/gpu/drm/tegra/hdmi.c  |  2 +-
  drivers/gpu/drm/tegra/sor.c   |  2 +-
  drivers/gpu/drm/vc4/vc4_hdmi.c|  2 +-
  drivers/gpu/drm/zte/zx_hdmi.c |  2 +-
  include/drm/drm_edid.h|  3 ++-
  21 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index daf003d..5dc3e95 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder 
*encoder,
dce_v10_0_audio_write_sad_regs(encoder);
dce_v10_0_audio_write_latency_fields(encoder, mode);
  
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);

+   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 3a72967..b70f077 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder 
*encoder,
dce_v11_0_audio_write_sad_regs(encoder);
dce_v11_0_audio_write_latency_fields(encoder, mode);
  
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);

+   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 6943f26..bcf9c75 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b

Re: [PATCH 01/11] drm: Add HDMI 2.0 VIC support for AVI info-frames

2017-04-10 Thread Andrzej Hajda
On 07.04.2017 18:39, Shashank Sharma wrote:
> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
> For any other mode, the VIC filed in AVI infoframes should be 0.
> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
> extended to (VIC 1-107).
>
> This patch adds a bool input variable, which indicates if the connected
> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
> HDMI 2.0 VIC to a HDMI 1.4 sink.
>
> This patch touches all drm drivers, who are callers of this function
> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
> no change in current behavior, is_hdmi2 is kept as false.
>
> In case of I915 driver, this patch checks the connector->display_info
> to check if the connected display is HDMI 2.0.
>
> Cc: Ville Syrjala 
> Cc: Jose Abreu 
> Cc: Andrzej Hajda 
> Cc: Alex Deucher 
> Cc: Daniel Vetter 
>
> PS: This patch touches a few lines in few files, which were
> already above 80 char, so checkpatch gives 80 char warning again.
> - gpu/drm/omapdrm/omap_encoder.c
> - gpu/drm/i915/intel_sdvo.c
>
> Signed-off-by: Shashank Sharma 

Finally I have chances to look at the specs and I am not sure if this
solution fully reflects the specs and is scalable. According to specs
VIC set to 0 in AVIF means "Video Format not documented in CTA-861", ie
it is described by vendor specific data block and vendor specific
infoframe, maybe something else(???). I suppose ideally during EDID read
there should be recorded info for every mode if it was defined by vendor
extension, or not. This info could be later used by drivers to configure
AVIF and VSIF accordingly.

Anyway as a short-term initial solution it could work.
So:
Reviewed-by: Andrzej Hajda 

 --
Regards
Andrzej

> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  2 +-
>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  3 ++-
>  drivers/gpu/drm/bridge/sii902x.c  |  2 +-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
>  drivers/gpu/drm/drm_edid.c| 12 +++-
>  drivers/gpu/drm/exynos/exynos_hdmi.c  |  2 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c |  2 +-
>  drivers/gpu/drm/i915/intel_hdmi.c |  5 -
>  drivers/gpu/drm/i915/intel_sdvo.c |  3 ++-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c   |  2 +-
>  drivers/gpu/drm/omapdrm/omap_encoder.c|  3 ++-
>  drivers/gpu/drm/radeon/radeon_audio.c |  2 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c  |  2 +-
>  drivers/gpu/drm/sti/sti_hdmi.c|  2 +-
>  drivers/gpu/drm/tegra/hdmi.c  |  2 +-
>  drivers/gpu/drm/tegra/sor.c   |  2 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c|  2 +-
>  drivers/gpu/drm/zte/zx_hdmi.c |  2 +-
>  include/drm/drm_edid.h|  3 ++-
>  21 files changed, 38 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index daf003d..5dc3e95 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder 
> *encoder,
>   dce_v10_0_audio_write_sad_regs(encoder);
>   dce_v10_0_audio_write_latency_fields(encoder, mode);
>  
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>   if (err < 0) {
>   DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
>   return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 3a72967..b70f077 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder 
> *encoder,
>   dce_v11_0_audio_write_sad_regs(encoder);
>   dce_v11_0_audio_write_latency_fields(encoder, mode);
>  
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>   if (err < 0) {
>   DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
>   return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index 6943f26..bcf9c75 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder 
> *encoder,
>   dce_v8_0_audio_write_sad_regs(encoder);
>   dce_v8_0_audio_write_latency_fields(encoder, mode);
>  
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>   if (err < 0) {
>   DRM_ERROR("failed to setup AVI inf

[PATCH 01/11] drm: Add HDMI 2.0 VIC support for AVI info-frames

2017-04-07 Thread Shashank Sharma
HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
For any other mode, the VIC filed in AVI infoframes should be 0.
HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
extended to (VIC 1-107).

This patch adds a bool input variable, which indicates if the connected
sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
HDMI 2.0 VIC to a HDMI 1.4 sink.

This patch touches all drm drivers, who are callers of this function
drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
no change in current behavior, is_hdmi2 is kept as false.

In case of I915 driver, this patch checks the connector->display_info
to check if the connected display is HDMI 2.0.

Cc: Ville Syrjala 
Cc: Jose Abreu 
Cc: Andrzej Hajda 
Cc: Alex Deucher 
Cc: Daniel Vetter 

PS: This patch touches a few lines in few files, which were
already above 80 char, so checkpatch gives 80 char warning again.
- gpu/drm/omapdrm/omap_encoder.c
- gpu/drm/i915/intel_sdvo.c

Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  2 +-
 drivers/gpu/drm/bridge/analogix-anx78xx.c |  3 ++-
 drivers/gpu/drm/bridge/sii902x.c  |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
 drivers/gpu/drm/drm_edid.c| 12 +++-
 drivers/gpu/drm/exynos/exynos_hdmi.c  |  2 +-
 drivers/gpu/drm/i2c/tda998x_drv.c |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c |  5 -
 drivers/gpu/drm/i915/intel_sdvo.c |  3 ++-
 drivers/gpu/drm/mediatek/mtk_hdmi.c   |  2 +-
 drivers/gpu/drm/omapdrm/omap_encoder.c|  3 ++-
 drivers/gpu/drm/radeon/radeon_audio.c |  2 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c  |  2 +-
 drivers/gpu/drm/sti/sti_hdmi.c|  2 +-
 drivers/gpu/drm/tegra/hdmi.c  |  2 +-
 drivers/gpu/drm/tegra/sor.c   |  2 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c|  2 +-
 drivers/gpu/drm/zte/zx_hdmi.c |  2 +-
 include/drm/drm_edid.h|  3 ++-
 21 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index daf003d..5dc3e95 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder 
*encoder,
dce_v10_0_audio_write_sad_regs(encoder);
dce_v10_0_audio_write_latency_fields(encoder, mode);
 
-   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 3a72967..b70f077 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder 
*encoder,
dce_v11_0_audio_write_sad_regs(encoder);
dce_v11_0_audio_write_latency_fields(encoder, mode);
 
-   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 6943f26..bcf9c75 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder 
*encoder,
dce_v8_0_audio_write_sad_regs(encoder);
dce_v8_0_audio_write_latency_fields(encoder, mode);
 
-   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
return;
diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c 
b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index a2a8236..f9b77b8 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_set(struct drm_bridge 
*bridge,
 
mutex_lock(&anx78xx->lock);
 
-   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode);
+   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
+  false);
if (err) {
DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
goto unlock;
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index