* [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer
@ 2017-07-13 15:33 Shashank Sharma
2017-07-13 15:33 ` [PATCH v2 01/14] drm: handle HDMI 2.0 VICs in AVI info-frames Shashank Sharma
` (16 more replies)
0 siblings, 17 replies; 30+ messages in thread
From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw)
To: intel-gfx, dri-devel
Following YCBCR 4:4:4 and 4:2:2, YCBCR 4:2:0 is a new output format,
which is currently supported on HDMI 2.0 sources/sinks. Due to lower
chroma sub-sampling rate, YCBCR 4:2:0 can drive the video modes at half
the pixel clock than YCBCR 4:4:4 or RGB 8:8:8 outputs. For example, a CEA
4K@60, RGB 8:8:8 mode needs a clock of appx 594Mhz, but it can be driven at
297Mhz using YCBCR 4:2:0 output.
Of course, the lower rate of chroma subsampling, causes the quality of YCBCR
4:2:0 to be lower than YCBCR 4:4:4 or RGB 8:8:8.
This patch series adds support for YCBCR 4:2:0 output in DRM layer.
- First 2 patches, complete the CEA mode-db in drm driver, by adding
new 4k modes. Current CEA mode-db contains 64 modes only (VIC 1-64),
whereas CEA-861-F defined VICs up to 107, including 4k modes, from VIC
range 93-107. First patch makes sure that inclusion of these modes doesn't
break existing HDMI 1.4 monitors, across various drivers.
- Next 5 patches focus on parsing new YCBCR 4:2:0 EDID blocks, and adding
YCBCR 4:2:0 modes in connector. They also contain a prune function, which
will cleanup the YCBCR 4:2:0 modes from list, if the connector doesn't
declare them supported.
- Next 2 patches add helper functions for identifing YCBCR 4:2:0 modes and
setup the color space in AVI infoframes.
- Next 6 patches add code for I915 layer handling of YCBCR 4:2:0 output.
This patch series was initially published as a complete framework to handle
all YCBCR outputs (4:4:4, 4:2:2, 4:2:0), but based on the code reviews, now
its been published as YCBCR 4:2:0 handling series only.
The previous discussion and reviews can be found here:
V5: https://patchwork.freedesktop.org/series/26815/
V1-V4: https://patchwork.freedesktop.org/series/22683/
Now re-publishing this patch series as YCBCR 4:2:0 handling series here.
V2: Addressed review comments from Ville
This series dropped one patch in V2(patch 8 from V1), so 14 patches in V2
This series has been tested with drm-tip code with following setup:
Source: Intel Geminilake device.
Sink: ACER S277HK HDMI 2.0 monitor.
Protocol testing: Astro VA-1844A HDMI analyzer.
Shashank Sharma (14):
drm: handle HDMI 2.0 VICs in AVI info-frames
drm/edid: complete CEA modedb(VIC 1-107)
drm/edid: parse sink information before CEA blocks
drm/edid: cleanup patch for CEA extended-tag macro
drm: add helper to validate YCBCR420 modes
drm/edid: parse YCBCR420 videomodes from EDID
drm/edid: parse ycbcr 420 deep color information
drm: add helper functions for YCBCR420 handling
drm/i915: add config function for YCBCR420 outputs
drm/i915: prepare scaler for YCBCR420 modeset
drm/i915: prepare pipe for YCBCR420 output
drm/i915: prepare csc unit for YCBCR420 output
drm/i915: set colorspace for YCBCR420 outputs
drm/i915/glk: set HDMI 2.0 identifier
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_v6_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 | 438 +++++++++++++++++++++++++++++-
drivers/gpu/drm/drm_modes.c | 87 ++++++
drivers/gpu/drm/drm_probe_helper.c | 4 +
drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 3 +
drivers/gpu/drm/i915/intel_color.c | 46 +++-
drivers/gpu/drm/i915/intel_display.c | 26 ++
drivers/gpu/drm/i915/intel_drv.h | 7 +-
drivers/gpu/drm/i915/intel_hdmi.c | 69 ++++-
drivers/gpu/drm/i915/intel_panel.c | 3 +-
drivers/gpu/drm/i915/intel_sdvo.c | 3 +-
drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 2 +-
drivers/gpu/drm/nouveau/nv50_display.c | 3 +-
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/sun4i/sun4i_hdmi_enc.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_connector.h | 32 +++
include/drm/drm_edid.h | 11 +-
include/drm/drm_modes.h | 11 +
34 files changed, 746 insertions(+), 39 deletions(-)
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH v2 01/14] drm: handle HDMI 2.0 VICs in AVI info-frames 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 02/14] drm/edid: complete CEA modedb(VIC 1-107) Shashank Sharma ` (15 subsequent siblings) 16 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel Cc: Jose Abreu, Andrzej Hajda, Alex Deucher, Daniel Vetter 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 if the connected display is HDMI 2.0. - HDMI infoframes carry one of this two type of information: - VIC for 4K modes for HDMI 1.4 sinks - S3D information for S3D modes As CEA-861-F has already defined VICs for 4K videomodes, this patch doesn't allow sending HDMI infoframes for HDMI 2.0 sinks, until the mode is 3D. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jose Abreu <jose.abreu@synopsys.com> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: Daniel Vetter <daniel.vetter@intel.com> 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 V2: Rebase, Added r-b from Andrzej V3: Addressed review comment from Ville: - Do not send VICs in both AVI-IF and HDMI-IF send only one of it. V4: Rebase V5: Added r-b from Neil. Addressed review comments from Ville - Do not block HDMI vendor IF, instead check for VIC while handling AVI infoframes V6: Rebase V7: Rebase Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- 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_v6_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 | 26 +++++++++++++++++++++++++- 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/msm/hdmi/hdmi_bridge.c | 2 +- drivers/gpu/drm/nouveau/nv50_display.c | 3 ++- 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/sun4i/sun4i_hdmi_enc.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 ++- 25 files changed, 57 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index 9f78c03..aff1f48 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -1867,7 +1867,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 4bcf01d..2df650d 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c @@ -1851,7 +1851,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_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c index fd134a4..0c3891f 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c @@ -1597,7 +1597,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder, ssize_t err; u32 tmp; - 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 a9e8695..c164bef 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c @@ -1750,7 +1750,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 c2fac39..dc045e0 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 b8d10e5..9efb7b8 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -269,7 +269,7 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge, if (ret) return; - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj); + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return; diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index de1308b..60faf2d 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -1317,7 +1317,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode) u8 val; /* Initialise info frame from DRM mode */ - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format)) frame.colorspace = HDMI_COLORSPACE_YUV444; diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2e55599..0667b07 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4334,12 +4334,14 @@ EXPORT_SYMBOL(drm_set_preferred_mode); * data from a DRM display mode * @frame: HDMI AVI infoframe * @mode: DRM display mode + * @is_hdmi2_sink: Sink is HDMI 2.0 compliant * * Return: 0 on success or a negative error code on failure. */ int drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, - const struct drm_display_mode *mode) + const struct drm_display_mode *mode, + bool is_hdmi2_sink) { int err; @@ -4355,6 +4357,28 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, frame->video_code = drm_match_cea_mode(mode); + /* + * HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but + * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we + * have to make sure we dont break HDMI 1.4 sinks. + */ + if (!is_hdmi2_sink && frame->video_code > 64) + frame->video_code = 0; + + /* + * HDMI spec says if a mode is found in HDMI 1.4b 4K modes + * we should send its VIC in vendor infoframes, else send the + * VIC in AVI infoframes. Lets check if this mode is present in + * HDMI 1.4b 4K modes + */ + if (frame->video_code) { + u8 vendor_if_vic = drm_match_hdmi_mode(mode); + bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK; + + if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d) + frame->video_code = 0; + } + frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; /* diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 06bfbe4..c953927 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -784,7 +784,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata) } ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, - &hdata->current_mode); + &hdata->current_mode, false); if (!ret) ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf)); if (ret > 0) { diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 86f47e1..d1e7ac5 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -712,7 +712,7 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode) { union hdmi_infoframe frame; - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode); + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL; tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index ec0779a..cc0d100 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -459,11 +459,14 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode; + struct drm_connector *connector = &intel_hdmi->attached_connector->base; + bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported; union hdmi_infoframe frame; int ret; ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, - adjusted_mode); + adjusted_mode, + is_hdmi2_sink); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return; diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index f902922..e58a47d 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -996,7 +996,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, ssize_t len; ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, - &pipe_config->base.adjusted_mode); + &pipe_config->base.adjusted_mode, + false); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return false; diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 0a4ffd7..5c0d024 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c @@ -975,7 +975,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi, u8 buffer[17]; ssize_t err; - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (err < 0) { dev_err(hdmi->dev, "Failed to get AVI infoframe from mode: %zd\n", err); diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index ae40e71..13ac822 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -97,7 +97,7 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi) u32 val; int len; - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode); + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); len = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer)); if (len < 0) { diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index 42a85c1..5f71e30 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -2762,7 +2762,8 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode) if (!drm_detect_hdmi_monitor(nv_connector->edid)) return; - ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, mode); + ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, mode, + false); if (!ret) { /* We have an AVI InfoFrame, populate it to the display */ args.pwr.avi_infoframe_length diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c index 86c977b..624f5b5 100644 --- a/drivers/gpu/drm/omapdrm/omap_encoder.c +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c @@ -85,7 +85,8 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder, if (hdmi_mode && dssdev->driver->set_hdmi_infoframe) { struct hdmi_avi_infoframe avi; - r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode); + r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode, + false); if (r == 0) dssdev->driver->set_hdmi_infoframe(dssdev, &avi); } diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c index aaacac1..770e31f 100644 --- a/drivers/gpu/drm/radeon/radeon_audio.c +++ b/drivers/gpu/drm/radeon/radeon_audio.c @@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder, if (!connector) return -EINVAL; - 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: %d\n", err); return err; diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c index 7d9b75e..7149968 100644 --- a/drivers/gpu/drm/rockchip/inno_hdmi.c +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c @@ -294,7 +294,7 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi, union hdmi_infoframe frame; int rc; - rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode); + rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444) frame.avi.colorspace = HDMI_COLORSPACE_YUV444; diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index a59c95a..dbc6a19 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -434,7 +434,7 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi) DRM_DEBUG_DRIVER("\n"); - ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode); + ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false); if (ret < 0) { DRM_ERROR("failed to setup AVI infoframe: %d\n", ret); return ret; diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c index d3398f62..83b7a2a 100644 --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c @@ -52,7 +52,7 @@ static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi, u8 buffer[17]; int i, ret; - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (ret < 0) { DRM_ERROR("Failed to get infoframes from mode\n"); return ret; diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c index cda0491..718d8db 100644 --- a/drivers/gpu/drm/tegra/hdmi.c +++ b/drivers/gpu/drm/tegra/hdmi.c @@ -734,7 +734,7 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi, u8 buffer[17]; ssize_t err; - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (err < 0) { dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index a8f5289..fb2709c 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -1904,7 +1904,7 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor, value &= ~INFOFRAME_CTRL_ENABLE; tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); if (err < 0) { dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err); return err; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index ed63d4e..406d6d8 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -395,7 +395,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder) union hdmi_infoframe frame; int ret; - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode); + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return; diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c index 0df7366..7e834e3 100644 --- a/drivers/gpu/drm/zte/zx_hdmi.c +++ b/drivers/gpu/drm/zte/zx_hdmi.c @@ -124,7 +124,7 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi, union hdmi_infoframe frame; int ret; - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode); + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); if (ret) { DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n", ret); diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 7b9f48b..89c0062 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -343,7 +343,8 @@ drm_load_edid_firmware(struct drm_connector *connector) int drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, - const struct drm_display_mode *mode); + const struct drm_display_mode *mode, + bool is_hdmi2_sink); int drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, const struct drm_display_mode *mode); -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 02/14] drm/edid: complete CEA modedb(VIC 1-107) 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 01/14] drm: handle HDMI 2.0 VICs in AVI info-frames Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 03/14] drm/edid: parse sink information before CEA blocks Shashank Sharma ` (14 subsequent siblings) 16 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel Cc: Jose Abreu, Andrzej Hajda, Alex Deucher, Harry Wentland CEA-861-F specs defines new video modes to be used with HDMI 2.0 EDIDs. The VIC range has been extended from 1-64 to 1-107. Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now to be able to parse new CEA modes using the existing methods, we have to complete the modedb (VIC=65 onwards). This patch adds: - Timings for existing CEA video modes (from VIC=65 till VIC=92) - Newly added 4k modes (from VIC=93 to VIC=107). The patch was originaly discussed and reviewed here: https://patchwork.freedesktop.org/patch/135810/ Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jose Abreu <Jose.Abreu@synopsys.com> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: Harry Wentland <harry.wentland@amd.com> V2: Rebase V3: Rebase V4: Added native bit handling as per CEA-861-F spec (Ville) V5: Fix timings for VIC 77:1920x1080 and 104:3840x2160p (Ville) Remove unnecessary paranthesis from function svd_to_vic (Ville) Added r-b (Neil) V6: Rebase V7: Fix indentation for modes from VIC 80 Reviewed-by: Jose Abreu <Jose.Abreu@synopsys.com> Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> Acked-by: Harry Wentland <harry.wentland@amd.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_edid.c | 227 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 226 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 0667b07..8de3092 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1006,6 +1006,221 @@ static const struct drm_display_mode edid_cea_modes[] = { 2492, 2640, 0, 1080, 1084, 1089, 1125, 0, DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, + /* 65 - 1280x720@24Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 59400, 1280, 3040, + 3080, 3300, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 66 - 1280x720@25Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3700, + 3740, 3960, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 67 - 1280x720@30Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3040, + 3080, 3300, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 68 - 1280x720@50Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1720, + 1760, 1980, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 69 - 1280x720@60Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390, + 1430, 1650, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 70 - 1280x720@100Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1720, + 1760, 1980, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 71 - 1280x720@120Hz */ + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1390, + 1430, 1650, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 72 - 1920x1080@24Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2558, + 2602, 2750, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 73 - 1920x1080@25Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2448, + 2492, 2640, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 74 - 1920x1080@30Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2008, + 2052, 2200, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 75 - 1920x1080@50Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2448, + 2492, 2640, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 76 - 1920x1080@60Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2008, + 2052, 2200, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 77 - 1920x1080@100Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 297000, 1920, 2448, + 2492, 2640, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 78 - 1920x1080@120Hz */ + { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 297000, 1920, 2008, + 2052, 2200, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 79 - 1680x720@24Hz */ + { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 59400, 1680, 3040, + 3080, 3300, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 80 - 1680x720@25Hz */ + { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 59400, 1680, 2908, + 2948, 3168, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 81 - 1680x720@30Hz */ + { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 59400, 1680, 2380, + 2420, 2640, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 82 - 1680x720@50Hz */ + { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 82500, 1680, 1940, + 1980, 2200, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 83 - 1680x720@60Hz */ + { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 99000, 1680, 1940, + 1980, 2200, 0, 720, 725, 730, 750, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 84 - 1680x720@100Hz */ + { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 165000, 1680, 1740, + 1780, 2000, 0, 720, 725, 730, 825, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 85 - 1680x720@120Hz */ + { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 198000, 1680, 1740, + 1780, 2000, 0, 720, 725, 730, 825, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 86 - 2560x1080@24Hz */ + { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 99000, 2560, 3558, + 3602, 3750, 0, 1080, 1084, 1089, 1100, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 87 - 2560x1080@25Hz */ + { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 90000, 2560, 3008, + 3052, 3200, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 88 - 2560x1080@30Hz */ + { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 118800, 2560, 3328, + 3372, 3520, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 89 - 2560x1080@50Hz */ + { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 185625, 2560, 3108, + 3152, 3300, 0, 1080, 1084, 1089, 1125, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 90 - 2560x1080@60Hz */ + { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 198000, 2560, 2808, + 2852, 3000, 0, 1080, 1084, 1089, 1100, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 91 - 2560x1080@100Hz */ + { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 371250, 2560, 2778, + 2822, 2970, 0, 1080, 1084, 1089, 1250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 92 - 2560x1080@120Hz */ + { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 495000, 2560, 3108, + 3152, 3300, 0, 1080, 1084, 1089, 1250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, }, + /* 93 - 3840x2160p@24Hz 16:9 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 5116, + 5204, 5500, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9,}, + /* 94 - 3840x2160p@25Hz 16:9 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4896, + 4984, 5280, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9}, + /* 95 - 3840x2160p@30Hz 16:9 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016, + 4104, 4400, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9}, + /* 96 - 3840x2160p@50Hz 16:9 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4896, + 4984, 5280, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9}, + /* 97 - 3840x2160p@60Hz 16:9 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4016, + 4104, 4400, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9}, + /* 98 - 4096x2160p@24Hz 256:135 */ + { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000, 4096, 5116, + 5204, 5500, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135}, + /* 99 - 4096x2160p@25Hz 256:135 */ + { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000, 4096, 5064, + 5152, 5280, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135}, + /* 100 - 4096x2160p@30Hz 256:135 */ + { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000, 4096, 4184, + 4272, 4400, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135}, + /* 101 - 4096x2160p@50Hz 256:135 */ + { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 594000, 4096, 5064, + 5152, 5280, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135}, + /* 102 - 4096x2160p@60Hz 256:135 */ + { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 594000, 4096, 4184, + 4272, 4400, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135}, + /* 103 - 3840x2160p@24Hz 64:27 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 5116, + 5204, 5500, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27}, + /* 104 - 3840x2160p@25Hz 64:27 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4896, + 4984, 5280, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27}, + /* 105 - 3840x2160p@30Hz 64:27 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016, + 4104, 4400, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27}, + /* 106 - 3840x2160p@50Hz 64:27 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4896, + 4984, 5280, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27}, + /* 107 - 3840x2160p@60Hz 64:27 */ + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4016, + 4104, 4400, 0, 2160, 2168, 2178, 2250, 0, + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), + .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27}, }; /* @@ -2902,6 +3117,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) return modes; } +static u8 svd_to_vic(u8 svd) +{ + + /* 0-6 bit vic, 7th bit native mode indicator */ + if ((svd >= 1 && svd <= 64) || (svd >= 129 && svd <= 192)) + return svd & 127; + + return svd; +} + static struct drm_display_mode * drm_display_mode_from_vic_index(struct drm_connector *connector, const u8 *video_db, u8 video_len, @@ -2915,7 +3140,7 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, return NULL; /* CEA modes are numbered 1..127 */ - vic = (video_db[video_index] & 127); + vic = svd_to_vic(video_db[video_index]); if (!drm_valid_cea_vic(vic)) return NULL; -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 03/14] drm/edid: parse sink information before CEA blocks 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 01/14] drm: handle HDMI 2.0 VICs in AVI info-frames Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 02/14] drm/edid: complete CEA modedb(VIC 1-107) Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 04/14] drm/edid: cleanup patch for CEA extended-tag macro Shashank Sharma ` (13 subsequent siblings) 16 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks. This block contains a map of indexes of CEA modes, which can support YCBCR 420 output also. To avoid multiple parsing of same CEA block, let's parse the sink information and get this map, before parsing CEA modes. This patch moves the call to drm_add_display_info function, before the mode parsing block. V4: Introduced new patch in the series V5: Move this patch before 4:2:0 parsing patch (ville) Added r-b from Ville V6: Rebase V7: Rebase Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_edid.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 8de3092..57c09d2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4440,6 +4440,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) quirks = edid_get_quirks(edid); /* + * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks. + * To avoid multiple parsing of same block, lets parse that map + * from sink info, before parsing CEA modes. + */ + drm_add_display_info(connector, edid); + + /* * EDID spec says modes should be preferred in this order: * - preferred detailed mode * - other detailed modes from base block @@ -4466,8 +4473,6 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75)) edid_fixup_preferred(connector, quirks); - drm_add_display_info(connector, edid); - if (quirks & EDID_QUIRK_FORCE_6BPC) connector->display_info.bpc = 6; -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 04/14] drm/edid: cleanup patch for CEA extended-tag macro 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (2 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 03/14] drm/edid: parse sink information before CEA blocks Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 05/14] drm: add helper to validate YCBCR420 modes Shashank Sharma ` (12 subsequent siblings) 16 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel CEA-861-F introduces extended tag codes for EDID extension blocks, which indicates the actual type of the data block. The code for using exteded tag is 0x7, whereas in the existing code, the corresponding macro is named as "VIDEO_CAPABILITY_BLOCK" This patch renames the macro and usages from "VIDEO_CAPABILITY_BLOCK" to "USE_EXTENDED_TAG" V2: Add extended tag code check for video capabilitiy block (ville) V3: Ville: - Use suggested names for macros - Check the block length first, before checking the extended tag V4: Fix commit message (David) V5: Introduced this patch into HDMI-YCBCR-output series V6: Rebase V7: Rebase Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_edid.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 57c09d2..16f73c5 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2781,7 +2781,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, #define VIDEO_BLOCK 0x02 #define VENDOR_BLOCK 0x03 #define SPEAKER_BLOCK 0x04 -#define VIDEO_CAPABILITY_BLOCK 0x07 +#define USE_EXTENDED_TAG 0x07 +#define EXT_VIDEO_CAPABILITY_BLOCK 0x00 #define EDID_BASIC_AUDIO (1 << 6) #define EDID_CEA_YCRCB444 (1 << 5) #define EDID_CEA_YCRCB422 (1 << 4) @@ -3443,6 +3444,12 @@ cea_db_payload_len(const u8 *db) } static int +cea_db_extended_tag(const u8 *db) +{ + return db[1]; +} + +static int cea_db_tag(const u8 *db) { return db[0] >> 5; @@ -4018,8 +4025,10 @@ bool drm_rgb_quant_range_selectable(struct edid *edid) return false; for_each_cea_db(edid_ext, i, start, end) { - if (cea_db_tag(&edid_ext[i]) == VIDEO_CAPABILITY_BLOCK && - cea_db_payload_len(&edid_ext[i]) == 2) { + if (cea_db_tag(&edid_ext[i]) == USE_EXTENDED_TAG && + cea_db_payload_len(&edid_ext[i]) == 2 && + cea_db_extended_tag(&edid_ext[i]) == + EXT_VIDEO_CAPABILITY_BLOCK) { DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", edid_ext[i + 2]); return edid_ext[i + 2] & EDID_CEA_VCDB_QS; } -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 05/14] drm: add helper to validate YCBCR420 modes 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (3 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 04/14] drm/edid: cleanup patch for CEA extended-tag macro Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID Shashank Sharma ` (11 subsequent siblings) 16 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel YCBCR420 modes are supported only on HDMI 2.0 capable sources. This patch adds: - A drm helper to validate YCBCR420-only mode on a particular connector. This function will help pruning the YCBCR420-only modes from the connector's modelist. - A bool variable (ycbcr_420_allowed) in the drm connector structure. While handling the EDID from HDMI 2.0 sinks, its important to know if the source is capable of handling YCBCR420 output, so that no YCBCR 420 modes will be listed for sources which can't handle it. A driver should set this variable if it wants to see YCBCR420 modes in the modedb. V5: Introduced the patch in series. V6: Squashed two patches (validate YCBCR420 and add YCBCR420 identifier) V7: Addressed review comments from Vile: - Move this patch before we add 420 modes from EDID. - No need for drm_valid_cea_vic() check, function back to non-static. - Update MODE_STATUS with NO_420 condition. - Introduce y420_vdb_modes variable in this patch Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_edid.c | 1 + drivers/gpu/drm/drm_modes.c | 29 +++++++++++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 4 ++++ include/drm/drm_connector.h | 17 +++++++++++++++++ include/drm/drm_modes.h | 5 +++++ 5 files changed, 56 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 16f73c5..96eee5a 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2952,6 +2952,7 @@ static bool drm_valid_cea_vic(u8 vic) { return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes); } +EXPORT_SYMBOL(drm_valid_cea_vic); /** * drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index f2493b9..35630b8 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1083,6 +1083,34 @@ drm_mode_validate_size(const struct drm_display_mode *mode, } EXPORT_SYMBOL(drm_mode_validate_size); +/** + * drm_mode_validate_ycbcr420 - add 'ycbcr420-only' modes only when allowed + * @mode: mode to check + * @connector: drm connector under action + * + * This function is a helper which can be used to filter out any YCBCR420 + * only mode, when the source doesn't support it. + * + * Returns: + * The mode status + */ +enum drm_mode_status +drm_mode_validate_ycbcr420(const struct drm_display_mode *mode, + struct drm_connector *connector) +{ + u8 vic = drm_match_cea_mode(mode); + enum drm_mode_status status = MODE_OK; + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; + + if (test_bit(vic, hdmi->y420_vdb_modes)) { + if (!connector->ycbcr_420_allowed) + status = MODE_NO_420; + } + + return status; +} +EXPORT_SYMBOL(drm_mode_validate_ycbcr420); + #define MODE_STATUS(status) [MODE_ ## status + 3] = #status static const char * const drm_mode_status_names[] = { @@ -1122,6 +1150,7 @@ static const char * const drm_mode_status_names[] = { MODE_STATUS(ONE_SIZE), MODE_STATUS(NO_REDUCED), MODE_STATUS(NO_STEREO), + MODE_STATUS(NO_420), MODE_STATUS(STALE), MODE_STATUS(BAD), MODE_STATUS(ERROR), diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 00e6832..904966c 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -528,6 +528,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, if (mode->status == MODE_OK) mode->status = drm_mode_validate_pipeline(mode, connector); + + if (mode->status == MODE_OK) + mode->status = drm_mode_validate_ycbcr420(mode, + connector); } prune: diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index ae5b7dc..26dd3eb 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -135,6 +135,14 @@ struct drm_scdc { struct drm_hdmi_info { /** @scdc: sink's scdc support and capabilities */ struct drm_scdc scdc; + + /** + * @y420_vdb_modes: bitmap of modes which can support ycbcr420 + * output only (not normal RGB/YCBCR444/422 outputs). There are total + * 107 VICs defined by CEA-861-F spec, so the size is 128 bits to map + * upto 128 VICs; + */ + unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; }; /** @@ -726,6 +734,15 @@ struct drm_connector { bool interlace_allowed; bool doublescan_allowed; bool stereo_allowed; + + /** + * @ycbcr_420_allowed : This bool indicates if this connector is + * capable of handling YCBCR 420 output. While parsing the EDID + * blocks, its very helpful to know, if the source is capable of + * handling YCBCR 420 outputs. + */ + bool ycbcr_420_allowed; + /** * @registered: Is this connector exposed (registered) with userspace? * Protected by @mutex. diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 94ac771..f8a1268 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -80,6 +80,7 @@ struct videomode; * @MODE_ONE_SIZE: only one resolution is supported * @MODE_NO_REDUCED: monitor doesn't accept reduced blanking * @MODE_NO_STEREO: stereo modes not supported + * @MODE_NO_420: ycbcr 420 modes not supported * @MODE_STALE: mode has become stale * @MODE_BAD: unspecified reason * @MODE_ERROR: error condition @@ -124,6 +125,7 @@ enum drm_mode_status { MODE_ONE_SIZE, MODE_NO_REDUCED, MODE_NO_STEREO, + MODE_NO_420, MODE_STALE = -3, MODE_BAD = -2, MODE_ERROR = -1 @@ -496,6 +498,9 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, enum drm_mode_status drm_mode_validate_basic(const struct drm_display_mode *mode); enum drm_mode_status drm_mode_validate_size(const struct drm_display_mode *mode, int maxX, int maxY); +enum drm_mode_status +drm_mode_validate_ycbcr420(const struct drm_display_mode *mode, + struct drm_connector *connector); void drm_mode_prune_invalid(struct drm_device *dev, struct list_head *mode_list, bool verbose); void drm_mode_sort(struct list_head *mode_list); -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (4 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 05/14] drm: add helper to validate YCBCR420 modes Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 16:21 ` Ville Syrjälä 2017-07-13 15:33 ` [PATCH v2 07/14] drm/edid: parse ycbcr 420 deep color information Shashank Sharma ` (10 subsequent siblings) 16 siblings, 1 reply; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel; +Cc: Jose Abreu, Emil Velikov HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. CEA-861-F adds two new blocks in EDID's CEA extension blocks, to provide information about sink's YCBCR420 output capabilities. These blocks are: - YCBCR420vdb(YCBCR 420 video data block): This block contains VICs of video modes, which can be sopported only in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal SVD block, valid for YCBCR420 modes only. - YCBCR420cmdb(YCBCR 420 capability map data block): This block gives information about video modes which can support YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This block contains a bitmap index of normal svd videomodes, which can support YCBCR420 output too. So if bit 0 from first vcb byte is set, first video mode in the svd list can support YCBCR420 output too. Bit 1 means second video mode from svd list can support YCBCR420 output too, and so on. This patch adds two bitmaps in display's hdmi_info structure, one each for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch adds: - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes an entry in the vdb_bitmap per vic. - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jose Abreu <joabreu@synopsys.com> Cc: Emil Velikov <emil.l.velikov@gmail.com> V2: Addressed Review comments from Emil: - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit. - Use the suggested method for updating dbmap. - Add documentation for YCBCR420_vcb_map to fix kbuild warning. Review comments from Ville: - Do not expose the YCBCR420 flags in uabi layer, keep it internal. - Save a map of YCBCR420 modes for future reference. - Check db length before trying to parse extended tag. - Add a warning if there are > 64 modes in capability map block. - Use y420cmdb in function names and macros while dealing with vcb to be aligned with spec. - Move the display information parsing block ahead of mode parsing blocks. V3: Addressed design/review comments from Ville - Do not add flags in video modes, else we have to expose them to user - There should not be a UABI change, and kernel should detect the choice of the output based on type of mode, and the bitmaps. - Use standard bitops from kernel bitmap header, instead of calculating bit positions manually. V4: Addressed review comments from Ville: - s/ycbcr_420_vdb/y420vdb - s/ycbcr_420_vcb/y420cmdb - Be less verbose on description of do_y420vdb_modes - Move newmode variable in the loop scope. - Use svd_to_vic() to get a VIC, instead of 0x7f - Remove bitmap description for CMDB modes & VDB modes - Dont add connector->ycbcr_420_allowed check for cmdb modes - Remove 'len' variable, in is_y420cmdb function, which is used only once - Add length check in is_y420vdb function - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap - Do not add print about YCBCR 420 modes - Fix indentation in few places - Move ycbcr420_dc_modes in next patch, where its used - Add a separate patch for movement of drm_add_display_info() V5: Addressed review comments from Ville: - Add the patch which cleans up the current EXTENDED_TAG usage - Make y420_cmdb_map u64 - Do not block ycbcr420 modes while parsing the EDID, rather add a separate helper function to prune ycbcr420-only modes from connector's probed modes. V6: Rebase V7: Move this patch after the 420_only validation patch (Ville) Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_edid.c | 148 +++++++++++++++++++++++++++++++++++++++++++- include/drm/drm_connector.h | 12 ++++ 2 files changed, 158 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 96eee5a..b86afb9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2783,6 +2783,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, #define SPEAKER_BLOCK 0x04 #define USE_EXTENDED_TAG 0x07 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 +#define EXT_VIDEO_DATA_BLOCK_420 0x0E +#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F #define EDID_BASIC_AUDIO (1 << 6) #define EDID_CEA_YCRCB444 (1 << 5) #define EDID_CEA_YCRCB422 (1 << 4) @@ -3155,15 +3157,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, return newmode; } +/* + * do_y420vdb_modes - Parse YCBCR 420 only modes + * @connector: connector corresponding to the HDMI sink + * @svds: start of the data block of CEA YCBCR 420 VDB + * @len: length of the CEA YCBCR 420 VDB + * + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB) + * which contains modes which can be supported in YCBCR 420 + * output format only. + */ +static int do_y420vdb_modes(struct drm_connector *connector, + const u8 *svds, u8 svds_len) +{ + int modes = 0, i; + struct drm_device *dev = connector->dev; + struct drm_display_info *info = &connector->display_info; + struct drm_hdmi_info *hdmi = &info->hdmi; + + for (i = 0; i < svds_len; i++) { + u8 vic = svd_to_vic(svds[i]); + struct drm_display_mode *newmode; + + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); + if (!newmode) + break; + bitmap_set(hdmi->y420_vdb_modes, vic, 1); + drm_mode_probed_add(connector, newmode); + modes++; + } + + if (modes > 0) + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; + return modes; +} + +/* + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap + * @connector: connector corresponding to the HDMI sink + * @vic: CEA vic for the video mode to be added in the map + * + * Makes an entry for a videomode in the YCBCR 420 bitmap + */ +static void +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd) +{ + u8 vic = svd_to_vic(svd); + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; + + bitmap_set(hdmi->y420_cmdb_modes, vic, 1); +} + static int do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) { int i, modes = 0; + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; for (i = 0; i < len; i++) { struct drm_display_mode *mode; mode = drm_display_mode_from_vic_index(connector, db, len, i); if (mode) { + /* + * YCBCR420 capability block contains a bitmap which + * gives the index of CEA modes from CEA VDB, which + * can support YCBCR 420 sampling output also (apart + * from RGB/YCBCR444 etc). + * For example, if the bit 0 in bitmap is set, + * first mode in VDB can support YCBCR420 output too. + * Add YCBCR420 modes only if sink is HDMI 2.0 capable. + */ + if (hdmi->y420_cmdb_map & (1 << i)) + drm_add_cmdb_modes(connector, db[i]); + drm_mode_probed_add(connector, mode); modes++; } @@ -3505,9 +3571,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) return oui == HDMI_FORUM_IEEE_OUI; } +static bool cea_db_is_y420cmdb(const u8 *db) +{ + + if (cea_db_tag(db) != USE_EXTENDED_TAG) + return false; + + if (!cea_db_payload_len(db)) + return false; + + if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB) + return false; + + return true; +} + +static bool cea_db_is_y420vdb(const u8 *db) +{ + if (cea_db_tag(db) != USE_EXTENDED_TAG) + return false; + + if (!cea_db_payload_len(db)) + return false; + + if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420) + return false; + + return true; +} + #define for_each_cea_db(cea, i, start, end) \ for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, + const u8 *db) +{ + struct drm_display_info *info = &connector->display_info; + struct drm_hdmi_info *hdmi = &info->hdmi; + u8 map_len = cea_db_payload_len(db) - 1; + u8 count; + u64 map = 0; + + if (map_len == 0) { + /* All CEA modes support ycbcr420 sampling also.*/ + hdmi->y420_cmdb_map = U64_MAX; + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; + return; + } + + /* + * This map indicates which of the existing CEA block modes + * from VDB can support YCBCR420 output too. So if bit=0 is + * set, first mode from VDB can support YCBCR420 output too. + * We will parse and keep this map, before parsing VDB itself + * to avoid going through the same block again and again. + * + * Spec is not clear about max possible size of this block. + * Clamping max bitmap block size at 8 bytes. Every byte can + * address 8 CEA modes, in this way this map can address + * 8*8 = first 64 SVDs. + */ + if (WARN_ON_ONCE(map_len > 8)) + map_len = 8; + + for (count = 0; count < map_len; count++) + map |= (u64)db[2 + count] << (8 * count); + + if (map) + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; + + hdmi->y420_cmdb_map = map; +} + static int add_cea_modes(struct drm_connector *connector, struct edid *edid) { @@ -3530,10 +3665,16 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) video = db + 1; video_len = dbl; modes += do_cea_modes(connector, video, dbl); - } - else if (cea_db_is_hdmi_vsdb(db)) { + } else if (cea_db_is_hdmi_vsdb(db)) { hdmi = db; hdmi_len = dbl; + } else if (cea_db_is_y420vdb(db)) { + const u8 *vdb420 = &db[2]; + + /* Add 4:2:0(only) modes present in EDID */ + modes += do_y420vdb_modes(connector, + vdb420, + dbl - 1); } } } @@ -4216,6 +4357,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, drm_parse_hdmi_vsdb_video(connector, db); if (cea_db_is_hdmi_forum_vsdb(db)) drm_parse_hdmi_forum_vsdb(connector, db); + if (cea_db_is_y420cmdb(db)) + drm_parse_y420cmdb_bitmap(connector, db); } } @@ -4477,6 +4620,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) num_modes += add_cea_modes(connector, edid); num_modes += add_alternate_cea_modes(connector, edid); num_modes += add_displayid_detailed_modes(connector, edid); + if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) num_modes += add_inferred_modes(connector, edid); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 26dd3eb..225e092 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -143,6 +143,17 @@ struct drm_hdmi_info { * upto 128 VICs; */ unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; + + /** + * @y420_cmdb_modes: bitmap of modes which can support ycbcr420 + * output also, along with normal HDMI outputs. There are total 107 + * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto + * 128 VICs; + */ + unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)]; + + /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ + u64 y420_cmdb_map; }; /** @@ -206,6 +217,7 @@ struct drm_display_info { #define DRM_COLOR_FORMAT_RGB444 (1<<0) #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) +#define DRM_COLOR_FORMAT_YCRCB420 (1<<3) /** * @color_formats: HDMI Color formats, selects between RGB and YCrCb -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID 2017-07-13 15:33 ` [PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID Shashank Sharma @ 2017-07-13 16:21 ` Ville Syrjälä 2017-07-14 4:04 ` Sharma, Shashank 0 siblings, 1 reply; 30+ messages in thread From: Ville Syrjälä @ 2017-07-13 16:21 UTC (permalink / raw) To: Shashank Sharma; +Cc: intel-gfx, Jose Abreu, Emil Velikov, dri-devel On Thu, Jul 13, 2017 at 09:03:12PM +0530, Shashank Sharma wrote: > HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. > CEA-861-F adds two new blocks in EDID's CEA extension blocks, > to provide information about sink's YCBCR420 output capabilities. > > These blocks are: > > - YCBCR420vdb(YCBCR 420 video data block): > This block contains VICs of video modes, which can be sopported only > in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal > SVD block, valid for YCBCR420 modes only. > > - YCBCR420cmdb(YCBCR 420 capability map data block): > This block gives information about video modes which can support > YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This > block contains a bitmap index of normal svd videomodes, which can > support YCBCR420 output too. > So if bit 0 from first vcb byte is set, first video mode in the svd > list can support YCBCR420 output too. Bit 1 means second video mode > from svd list can support YCBCR420 output too, and so on. > > This patch adds two bitmaps in display's hdmi_info structure, one each > for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch > adds: > - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes > an entry in the vdb_bitmap per vic. > - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Jose Abreu <joabreu@synopsys.com> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > > V2: Addressed > Review comments from Emil: > - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit. > - Use the suggested method for updating dbmap. > - Add documentation for YCBCR420_vcb_map to fix kbuild warning. > > Review comments from Ville: > - Do not expose the YCBCR420 flags in uabi layer, keep it internal. > - Save a map of YCBCR420 modes for future reference. > - Check db length before trying to parse extended tag. > - Add a warning if there are > 64 modes in capability map block. > - Use y420cmdb in function names and macros while dealing with vcb > to be aligned with spec. > - Move the display information parsing block ahead of mode parsing > blocks. > > V3: Addressed design/review comments from Ville > - Do not add flags in video modes, else we have to expose them to user > - There should not be a UABI change, and kernel should detect the > choice of the output based on type of mode, and the bitmaps. > - Use standard bitops from kernel bitmap header, instead of calculating > bit positions manually. > > V4: Addressed review comments from Ville: > - s/ycbcr_420_vdb/y420vdb > - s/ycbcr_420_vcb/y420cmdb > - Be less verbose on description of do_y420vdb_modes > - Move newmode variable in the loop scope. > - Use svd_to_vic() to get a VIC, instead of 0x7f > - Remove bitmap description for CMDB modes & VDB modes > - Dont add connector->ycbcr_420_allowed check for cmdb modes > - Remove 'len' variable, in is_y420cmdb function, which is used > only once > - Add length check in is_y420vdb function > - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap > - Do not add print about YCBCR 420 modes > - Fix indentation in few places > - Move ycbcr420_dc_modes in next patch, where its used > - Add a separate patch for movement of drm_add_display_info() > > V5: Addressed review comments from Ville: > - Add the patch which cleans up the current EXTENDED_TAG usage > - Make y420_cmdb_map u64 > - Do not block ycbcr420 modes while parsing the EDID, rather > add a separate helper function to prune ycbcr420-only modes from > connector's probed modes. > > V6: Rebase > V7: Move this patch after the 420_only validation patch (Ville) > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/drm_edid.c | 148 +++++++++++++++++++++++++++++++++++++++++++- > include/drm/drm_connector.h | 12 ++++ > 2 files changed, 158 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 96eee5a..b86afb9 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2783,6 +2783,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, > #define SPEAKER_BLOCK 0x04 > #define USE_EXTENDED_TAG 0x07 > #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 > +#define EXT_VIDEO_DATA_BLOCK_420 0x0E > +#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F > #define EDID_BASIC_AUDIO (1 << 6) > #define EDID_CEA_YCRCB444 (1 << 5) > #define EDID_CEA_YCRCB422 (1 << 4) > @@ -3155,15 +3157,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, > return newmode; > } > > +/* > + * do_y420vdb_modes - Parse YCBCR 420 only modes > + * @connector: connector corresponding to the HDMI sink > + * @svds: start of the data block of CEA YCBCR 420 VDB > + * @len: length of the CEA YCBCR 420 VDB > + * > + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB) > + * which contains modes which can be supported in YCBCR 420 > + * output format only. > + */ > +static int do_y420vdb_modes(struct drm_connector *connector, > + const u8 *svds, u8 svds_len) > +{ > + int modes = 0, i; > + struct drm_device *dev = connector->dev; > + struct drm_display_info *info = &connector->display_info; > + struct drm_hdmi_info *hdmi = &info->hdmi; > + > + for (i = 0; i < svds_len; i++) { > + u8 vic = svd_to_vic(svds[i]); > + struct drm_display_mode *newmode; > + Hmm. Looks like here we will need the drm_valid_cea_vic() check because the vic is coming from an untrusted source. > + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); > + if (!newmode) > + break; > + bitmap_set(hdmi->y420_vdb_modes, vic, 1); > + drm_mode_probed_add(connector, newmode); > + modes++; > + } > + > + if (modes > 0) > + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; > + return modes; > +} > + > +/* > + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap > + * @connector: connector corresponding to the HDMI sink > + * @vic: CEA vic for the video mode to be added in the map > + * > + * Makes an entry for a videomode in the YCBCR 420 bitmap > + */ > +static void > +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd) > +{ > + u8 vic = svd_to_vic(svd); > + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; And maybe here. It's not actually needed since we know it's valid because drm_display_mode_from_vic_index() returned a mode, but since that's not immediately obvious from here we might want to have the check. But if you disagree feel free to leave it out. At some point I think we might want to reorganize drm_display_mode_from_vic_index() such that we actually pass it the vic instead. Then we could also pass the vic here directly. > + > + bitmap_set(hdmi->y420_cmdb_modes, vic, 1); > +} > + > static int > do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) > { > int i, modes = 0; > + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; > > for (i = 0; i < len; i++) { > struct drm_display_mode *mode; > mode = drm_display_mode_from_vic_index(connector, db, len, i); > if (mode) { > + /* > + * YCBCR420 capability block contains a bitmap which > + * gives the index of CEA modes from CEA VDB, which > + * can support YCBCR 420 sampling output also (apart > + * from RGB/YCBCR444 etc). > + * For example, if the bit 0 in bitmap is set, > + * first mode in VDB can support YCBCR420 output too. > + * Add YCBCR420 modes only if sink is HDMI 2.0 capable. > + */ > + if (hdmi->y420_cmdb_map & (1 << i)) I think this needs to be something like if (i < 64 && hdmi->y420_cmdb_map & (1ULL << i)) With those changes I think this part should be good to go. > + drm_add_cmdb_modes(connector, db[i]); > + > drm_mode_probed_add(connector, mode); > modes++; > } > @@ -3505,9 +3571,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) > return oui == HDMI_FORUM_IEEE_OUI; > } > > +static bool cea_db_is_y420cmdb(const u8 *db) > +{ > + > + if (cea_db_tag(db) != USE_EXTENDED_TAG) > + return false; > + > + if (!cea_db_payload_len(db)) > + return false; > + > + if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB) > + return false; > + > + return true; > +} > + > +static bool cea_db_is_y420vdb(const u8 *db) > +{ > + if (cea_db_tag(db) != USE_EXTENDED_TAG) > + return false; > + > + if (!cea_db_payload_len(db)) > + return false; > + > + if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420) > + return false; > + > + return true; > +} > + > #define for_each_cea_db(cea, i, start, end) \ > for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) > > +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, > + const u8 *db) > +{ > + struct drm_display_info *info = &connector->display_info; > + struct drm_hdmi_info *hdmi = &info->hdmi; > + u8 map_len = cea_db_payload_len(db) - 1; > + u8 count; > + u64 map = 0; > + > + if (map_len == 0) { > + /* All CEA modes support ycbcr420 sampling also.*/ > + hdmi->y420_cmdb_map = U64_MAX; > + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; > + return; > + } > + > + /* > + * This map indicates which of the existing CEA block modes > + * from VDB can support YCBCR420 output too. So if bit=0 is > + * set, first mode from VDB can support YCBCR420 output too. > + * We will parse and keep this map, before parsing VDB itself > + * to avoid going through the same block again and again. > + * > + * Spec is not clear about max possible size of this block. > + * Clamping max bitmap block size at 8 bytes. Every byte can > + * address 8 CEA modes, in this way this map can address > + * 8*8 = first 64 SVDs. > + */ > + if (WARN_ON_ONCE(map_len > 8)) > + map_len = 8; > + > + for (count = 0; count < map_len; count++) > + map |= (u64)db[2 + count] << (8 * count); > + > + if (map) > + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; > + > + hdmi->y420_cmdb_map = map; > +} > + > static int > add_cea_modes(struct drm_connector *connector, struct edid *edid) > { > @@ -3530,10 +3665,16 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) > video = db + 1; > video_len = dbl; > modes += do_cea_modes(connector, video, dbl); > - } > - else if (cea_db_is_hdmi_vsdb(db)) { > + } else if (cea_db_is_hdmi_vsdb(db)) { > hdmi = db; > hdmi_len = dbl; > + } else if (cea_db_is_y420vdb(db)) { > + const u8 *vdb420 = &db[2]; > + > + /* Add 4:2:0(only) modes present in EDID */ > + modes += do_y420vdb_modes(connector, > + vdb420, > + dbl - 1); > } > } > } > @@ -4216,6 +4357,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, > drm_parse_hdmi_vsdb_video(connector, db); > if (cea_db_is_hdmi_forum_vsdb(db)) > drm_parse_hdmi_forum_vsdb(connector, db); > + if (cea_db_is_y420cmdb(db)) > + drm_parse_y420cmdb_bitmap(connector, db); > } > } > > @@ -4477,6 +4620,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) > num_modes += add_cea_modes(connector, edid); > num_modes += add_alternate_cea_modes(connector, edid); > num_modes += add_displayid_detailed_modes(connector, edid); > + > if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) > num_modes += add_inferred_modes(connector, edid); > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 26dd3eb..225e092 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -143,6 +143,17 @@ struct drm_hdmi_info { > * upto 128 VICs; > */ > unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; > + > + /** > + * @y420_cmdb_modes: bitmap of modes which can support ycbcr420 > + * output also, along with normal HDMI outputs. There are total 107 > + * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto > + * 128 VICs; > + */ > + unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)]; > + > + /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ > + u64 y420_cmdb_map; > }; > > /** > @@ -206,6 +217,7 @@ struct drm_display_info { > #define DRM_COLOR_FORMAT_RGB444 (1<<0) > #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) > #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) > +#define DRM_COLOR_FORMAT_YCRCB420 (1<<3) > > /** > * @color_formats: HDMI Color formats, selects between RGB and YCrCb > -- > 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID 2017-07-13 16:21 ` Ville Syrjälä @ 2017-07-14 4:04 ` Sharma, Shashank 2017-07-14 10:33 ` [PATCH v8 " Shashank Sharma 0 siblings, 1 reply; 30+ messages in thread From: Sharma, Shashank @ 2017-07-14 4:04 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, Jose Abreu, dri-devel Regards Shashank On 7/13/2017 9:51 PM, Ville Syrjälä wrote: > On Thu, Jul 13, 2017 at 09:03:12PM +0530, Shashank Sharma wrote: >> HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. >> CEA-861-F adds two new blocks in EDID's CEA extension blocks, >> to provide information about sink's YCBCR420 output capabilities. >> >> These blocks are: >> >> - YCBCR420vdb(YCBCR 420 video data block): >> This block contains VICs of video modes, which can be sopported only >> in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal >> SVD block, valid for YCBCR420 modes only. >> >> - YCBCR420cmdb(YCBCR 420 capability map data block): >> This block gives information about video modes which can support >> YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This >> block contains a bitmap index of normal svd videomodes, which can >> support YCBCR420 output too. >> So if bit 0 from first vcb byte is set, first video mode in the svd >> list can support YCBCR420 output too. Bit 1 means second video mode >> from svd list can support YCBCR420 output too, and so on. >> >> This patch adds two bitmaps in display's hdmi_info structure, one each >> for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch >> adds: >> - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes >> an entry in the vdb_bitmap per vic. >> - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Jose Abreu <joabreu@synopsys.com> >> Cc: Emil Velikov <emil.l.velikov@gmail.com> >> >> V2: Addressed >> Review comments from Emil: >> - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit. >> - Use the suggested method for updating dbmap. >> - Add documentation for YCBCR420_vcb_map to fix kbuild warning. >> >> Review comments from Ville: >> - Do not expose the YCBCR420 flags in uabi layer, keep it internal. >> - Save a map of YCBCR420 modes for future reference. >> - Check db length before trying to parse extended tag. >> - Add a warning if there are > 64 modes in capability map block. >> - Use y420cmdb in function names and macros while dealing with vcb >> to be aligned with spec. >> - Move the display information parsing block ahead of mode parsing >> blocks. >> >> V3: Addressed design/review comments from Ville >> - Do not add flags in video modes, else we have to expose them to user >> - There should not be a UABI change, and kernel should detect the >> choice of the output based on type of mode, and the bitmaps. >> - Use standard bitops from kernel bitmap header, instead of calculating >> bit positions manually. >> >> V4: Addressed review comments from Ville: >> - s/ycbcr_420_vdb/y420vdb >> - s/ycbcr_420_vcb/y420cmdb >> - Be less verbose on description of do_y420vdb_modes >> - Move newmode variable in the loop scope. >> - Use svd_to_vic() to get a VIC, instead of 0x7f >> - Remove bitmap description for CMDB modes & VDB modes >> - Dont add connector->ycbcr_420_allowed check for cmdb modes >> - Remove 'len' variable, in is_y420cmdb function, which is used >> only once >> - Add length check in is_y420vdb function >> - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap >> - Do not add print about YCBCR 420 modes >> - Fix indentation in few places >> - Move ycbcr420_dc_modes in next patch, where its used >> - Add a separate patch for movement of drm_add_display_info() >> >> V5: Addressed review comments from Ville: >> - Add the patch which cleans up the current EXTENDED_TAG usage >> - Make y420_cmdb_map u64 >> - Do not block ycbcr420 modes while parsing the EDID, rather >> add a separate helper function to prune ycbcr420-only modes from >> connector's probed modes. >> >> V6: Rebase >> V7: Move this patch after the 420_only validation patch (Ville) >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/drm_edid.c | 148 +++++++++++++++++++++++++++++++++++++++++++- >> include/drm/drm_connector.h | 12 ++++ >> 2 files changed, 158 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index 96eee5a..b86afb9 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -2783,6 +2783,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, >> #define SPEAKER_BLOCK 0x04 >> #define USE_EXTENDED_TAG 0x07 >> #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 >> +#define EXT_VIDEO_DATA_BLOCK_420 0x0E >> +#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F >> #define EDID_BASIC_AUDIO (1 << 6) >> #define EDID_CEA_YCRCB444 (1 << 5) >> #define EDID_CEA_YCRCB422 (1 << 4) >> @@ -3155,15 +3157,79 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, >> return newmode; >> } >> >> +/* >> + * do_y420vdb_modes - Parse YCBCR 420 only modes >> + * @connector: connector corresponding to the HDMI sink >> + * @svds: start of the data block of CEA YCBCR 420 VDB >> + * @len: length of the CEA YCBCR 420 VDB >> + * >> + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB) >> + * which contains modes which can be supported in YCBCR 420 >> + * output format only. >> + */ >> +static int do_y420vdb_modes(struct drm_connector *connector, >> + const u8 *svds, u8 svds_len) >> +{ >> + int modes = 0, i; >> + struct drm_device *dev = connector->dev; >> + struct drm_display_info *info = &connector->display_info; >> + struct drm_hdmi_info *hdmi = &info->hdmi; >> + >> + for (i = 0; i < svds_len; i++) { >> + u8 vic = svd_to_vic(svds[i]); >> + struct drm_display_mode *newmode; >> + > Hmm. Looks like here we will need the drm_valid_cea_vic() check > because the vic is coming from an untrusted source. Agree, there should be one. >> + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); >> + if (!newmode) >> + break; >> + bitmap_set(hdmi->y420_vdb_modes, vic, 1); >> + drm_mode_probed_add(connector, newmode); >> + modes++; >> + } >> + >> + if (modes > 0) >> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; >> + return modes; >> +} >> + >> +/* >> + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap >> + * @connector: connector corresponding to the HDMI sink >> + * @vic: CEA vic for the video mode to be added in the map >> + * >> + * Makes an entry for a videomode in the YCBCR 420 bitmap >> + */ >> +static void >> +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd) >> +{ >> + u8 vic = svd_to_vic(svd); >> + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; > And maybe here. It's not actually needed since we know it's valid > because drm_display_mode_from_vic_index() returned a mode, but since > that's not immediately obvious from here we might want to have the > check. But if you disagree feel free to leave it out. I guess its a good idea to be careful while adding modes, I can add a check here too. > At some point I think we might want to reorganize > drm_display_mode_from_vic_index() such that we actually pass it the > vic instead. Then we could also pass the vic here directly. > >> + >> + bitmap_set(hdmi->y420_cmdb_modes, vic, 1); >> +} >> + >> static int >> do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) >> { >> int i, modes = 0; >> + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; >> >> for (i = 0; i < len; i++) { >> struct drm_display_mode *mode; >> mode = drm_display_mode_from_vic_index(connector, db, len, i); >> if (mode) { >> + /* >> + * YCBCR420 capability block contains a bitmap which >> + * gives the index of CEA modes from CEA VDB, which >> + * can support YCBCR 420 sampling output also (apart >> + * from RGB/YCBCR444 etc). >> + * For example, if the bit 0 in bitmap is set, >> + * first mode in VDB can support YCBCR420 output too. >> + * Add YCBCR420 modes only if sink is HDMI 2.0 capable. >> + */ >> + if (hdmi->y420_cmdb_map & (1 << i)) > I think this needs to be something like > if (i < 64 && hdmi->y420_cmdb_map & (1ULL << i)) Yeah, that's also one preventive measure, which we can add. - Shashank > > With those changes I think this part should be good to go. > >> + drm_add_cmdb_modes(connector, db[i]); >> + >> drm_mode_probed_add(connector, mode); >> modes++; >> } >> @@ -3505,9 +3571,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) >> return oui == HDMI_FORUM_IEEE_OUI; >> } >> >> +static bool cea_db_is_y420cmdb(const u8 *db) >> +{ >> + >> + if (cea_db_tag(db) != USE_EXTENDED_TAG) >> + return false; >> + >> + if (!cea_db_payload_len(db)) >> + return false; >> + >> + if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB) >> + return false; >> + >> + return true; >> +} >> + >> +static bool cea_db_is_y420vdb(const u8 *db) >> +{ >> + if (cea_db_tag(db) != USE_EXTENDED_TAG) >> + return false; >> + >> + if (!cea_db_payload_len(db)) >> + return false; >> + >> + if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420) >> + return false; >> + >> + return true; >> +} >> + >> #define for_each_cea_db(cea, i, start, end) \ >> for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) >> >> +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, >> + const u8 *db) >> +{ >> + struct drm_display_info *info = &connector->display_info; >> + struct drm_hdmi_info *hdmi = &info->hdmi; >> + u8 map_len = cea_db_payload_len(db) - 1; >> + u8 count; >> + u64 map = 0; >> + >> + if (map_len == 0) { >> + /* All CEA modes support ycbcr420 sampling also.*/ >> + hdmi->y420_cmdb_map = U64_MAX; >> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; >> + return; >> + } >> + >> + /* >> + * This map indicates which of the existing CEA block modes >> + * from VDB can support YCBCR420 output too. So if bit=0 is >> + * set, first mode from VDB can support YCBCR420 output too. >> + * We will parse and keep this map, before parsing VDB itself >> + * to avoid going through the same block again and again. >> + * >> + * Spec is not clear about max possible size of this block. >> + * Clamping max bitmap block size at 8 bytes. Every byte can >> + * address 8 CEA modes, in this way this map can address >> + * 8*8 = first 64 SVDs. >> + */ >> + if (WARN_ON_ONCE(map_len > 8)) >> + map_len = 8; >> + >> + for (count = 0; count < map_len; count++) >> + map |= (u64)db[2 + count] << (8 * count); >> + >> + if (map) >> + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; >> + >> + hdmi->y420_cmdb_map = map; >> +} >> + >> static int >> add_cea_modes(struct drm_connector *connector, struct edid *edid) >> { >> @@ -3530,10 +3665,16 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) >> video = db + 1; >> video_len = dbl; >> modes += do_cea_modes(connector, video, dbl); >> - } >> - else if (cea_db_is_hdmi_vsdb(db)) { >> + } else if (cea_db_is_hdmi_vsdb(db)) { >> hdmi = db; >> hdmi_len = dbl; >> + } else if (cea_db_is_y420vdb(db)) { >> + const u8 *vdb420 = &db[2]; >> + >> + /* Add 4:2:0(only) modes present in EDID */ >> + modes += do_y420vdb_modes(connector, >> + vdb420, >> + dbl - 1); >> } >> } >> } >> @@ -4216,6 +4357,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, >> drm_parse_hdmi_vsdb_video(connector, db); >> if (cea_db_is_hdmi_forum_vsdb(db)) >> drm_parse_hdmi_forum_vsdb(connector, db); >> + if (cea_db_is_y420cmdb(db)) >> + drm_parse_y420cmdb_bitmap(connector, db); >> } >> } >> >> @@ -4477,6 +4620,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) >> num_modes += add_cea_modes(connector, edid); >> num_modes += add_alternate_cea_modes(connector, edid); >> num_modes += add_displayid_detailed_modes(connector, edid); >> + >> if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) >> num_modes += add_inferred_modes(connector, edid); >> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >> index 26dd3eb..225e092 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -143,6 +143,17 @@ struct drm_hdmi_info { >> * upto 128 VICs; >> */ >> unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; >> + >> + /** >> + * @y420_cmdb_modes: bitmap of modes which can support ycbcr420 >> + * output also, along with normal HDMI outputs. There are total 107 >> + * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto >> + * 128 VICs; >> + */ >> + unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)]; >> + >> + /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ >> + u64 y420_cmdb_map; >> }; >> >> /** >> @@ -206,6 +217,7 @@ struct drm_display_info { >> #define DRM_COLOR_FORMAT_RGB444 (1<<0) >> #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) >> #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) >> +#define DRM_COLOR_FORMAT_YCRCB420 (1<<3) >> >> /** >> * @color_formats: HDMI Color formats, selects between RGB and YCrCb >> -- >> 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v8 06/14] drm/edid: parse YCBCR420 videomodes from EDID 2017-07-14 4:04 ` Sharma, Shashank @ 2017-07-14 10:33 ` Shashank Sharma 0 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-14 10:33 UTC (permalink / raw) To: intel-gfx, dri-devel; +Cc: Jose Abreu HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. CEA-861-F adds two new blocks in EDID's CEA extension blocks, to provide information about sink's YCBCR420 output capabilities. These blocks are: - YCBCR420vdb(YCBCR 420 video data block): This block contains VICs of video modes, which can be sopported only in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal SVD block, valid for YCBCR420 modes only. - YCBCR420cmdb(YCBCR 420 capability map data block): This block gives information about video modes which can support YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This block contains a bitmap index of normal svd videomodes, which can support YCBCR420 output too. So if bit 0 from first vcb byte is set, first video mode in the svd list can support YCBCR420 output too. Bit 1 means second video mode from svd list can support YCBCR420 output too, and so on. This patch adds two bitmaps in display's hdmi_info structure, one each for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch adds: - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes an entry in the vdb_bitmap per vic. - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jose Abreu <joabreu@synopsys.com> Cc: Emil Velikov <emil.l.velikov@gmail.com> V2: Addressed Review comments from Emil: - Use 1ULL<<i instead of 1<<i to make sure the output is 64bit. - Use the suggested method for updating dbmap. - Add documentation for YCBCR420_vcb_map to fix kbuild warning. Review comments from Ville: - Do not expose the YCBCR420 flags in uabi layer, keep it internal. - Save a map of YCBCR420 modes for future reference. - Check db length before trying to parse extended tag. - Add a warning if there are > 64 modes in capability map block. - Use y420cmdb in function names and macros while dealing with vcb to be aligned with spec. - Move the display information parsing block ahead of mode parsing blocks. V3: Addressed design/review comments from Ville - Do not add flags in video modes, else we have to expose them to user - There should not be a UABI change, and kernel should detect the choice of the output based on type of mode, and the bitmaps. - Use standard bitops from kernel bitmap header, instead of calculating bit positions manually. V4: Addressed review comments from Ville: - s/ycbcr_420_vdb/y420vdb - s/ycbcr_420_vcb/y420cmdb - Be less verbose on description of do_y420vdb_modes - Move newmode variable in the loop scope. - Use svd_to_vic() to get a VIC, instead of 0x7f - Remove bitmap description for CMDB modes & VDB modes - Dont add connector->ycbcr_420_allowed check for cmdb modes - Remove 'len' variable, in is_y420cmdb function, which is used only once - Add length check in is_y420vdb function - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap - Do not add print about YCBCR 420 modes - Fix indentation in few places - Move ycbcr420_dc_modes in next patch, where its used - Add a separate patch for movement of drm_add_display_info() V5: Addressed review comments from Ville: - Add the patch which cleans up the current EXTENDED_TAG usage - Make y420_cmdb_map u64 - Do not block ycbcr420 modes while parsing the EDID, rather add a separate helper function to prune ycbcr420-only modes from connector's probed modes. V6: Rebase V7: Move this patch after the 420_only validation patch (Ville) V8: Addressed review comments from Ville - use cea_vic_valid check before adding cmdb/vdb modes - add check for i < 64 while adding cmdb modes - use 1ULL while checking bitmap Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_edid.c | 154 +++++++++++++++++++++++++++++++++++++++++++- include/drm/drm_connector.h | 12 ++++ 2 files changed, 164 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 96eee5a..9fa0d7f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2783,6 +2783,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, #define SPEAKER_BLOCK 0x04 #define USE_EXTENDED_TAG 0x07 #define EXT_VIDEO_CAPABILITY_BLOCK 0x00 +#define EXT_VIDEO_DATA_BLOCK_420 0x0E +#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F #define EDID_BASIC_AUDIO (1 << 6) #define EDID_CEA_YCRCB444 (1 << 5) #define EDID_CEA_YCRCB422 (1 << 4) @@ -3155,15 +3157,85 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, return newmode; } +/* + * do_y420vdb_modes - Parse YCBCR 420 only modes + * @connector: connector corresponding to the HDMI sink + * @svds: start of the data block of CEA YCBCR 420 VDB + * @len: length of the CEA YCBCR 420 VDB + * + * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB) + * which contains modes which can be supported in YCBCR 420 + * output format only. + */ +static int do_y420vdb_modes(struct drm_connector *connector, + const u8 *svds, u8 svds_len) +{ + int modes = 0, i; + struct drm_device *dev = connector->dev; + struct drm_display_info *info = &connector->display_info; + struct drm_hdmi_info *hdmi = &info->hdmi; + + for (i = 0; i < svds_len; i++) { + u8 vic = svd_to_vic(svds[i]); + struct drm_display_mode *newmode; + + if (!drm_valid_cea_vic(vic)) + continue; + + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); + if (!newmode) + break; + bitmap_set(hdmi->y420_vdb_modes, vic, 1); + drm_mode_probed_add(connector, newmode); + modes++; + } + + if (modes > 0) + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; + return modes; +} + +/* + * drm_add_cmdb_modes - Add a YCBCR 420 mode into bitmap + * @connector: connector corresponding to the HDMI sink + * @vic: CEA vic for the video mode to be added in the map + * + * Makes an entry for a videomode in the YCBCR 420 bitmap + */ +static void +drm_add_cmdb_modes(struct drm_connector *connector, u8 svd) +{ + u8 vic = svd_to_vic(svd); + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; + + if (!drm_valid_cea_vic(vic)) + return; + + bitmap_set(hdmi->y420_cmdb_modes, vic, 1); +} + static int do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) { int i, modes = 0; + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; for (i = 0; i < len; i++) { struct drm_display_mode *mode; mode = drm_display_mode_from_vic_index(connector, db, len, i); if (mode) { + /* + * YCBCR420 capability block contains a bitmap which + * gives the index of CEA modes from CEA VDB, which + * can support YCBCR 420 sampling output also (apart + * from RGB/YCBCR444 etc). + * For example, if the bit 0 in bitmap is set, + * first mode in VDB can support YCBCR420 output too. + * Add YCBCR420 modes only if sink is HDMI 2.0 capable. + */ + if (i < 64 && hdmi->y420_cmdb_map & (1ULL << i)) + drm_add_cmdb_modes(connector, db[i]); + drm_mode_probed_add(connector, mode); modes++; } @@ -3505,9 +3577,78 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) return oui == HDMI_FORUM_IEEE_OUI; } +static bool cea_db_is_y420cmdb(const u8 *db) +{ + + if (cea_db_tag(db) != USE_EXTENDED_TAG) + return false; + + if (!cea_db_payload_len(db)) + return false; + + if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB) + return false; + + return true; +} + +static bool cea_db_is_y420vdb(const u8 *db) +{ + if (cea_db_tag(db) != USE_EXTENDED_TAG) + return false; + + if (!cea_db_payload_len(db)) + return false; + + if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420) + return false; + + return true; +} + #define for_each_cea_db(cea, i, start, end) \ for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1) +static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, + const u8 *db) +{ + struct drm_display_info *info = &connector->display_info; + struct drm_hdmi_info *hdmi = &info->hdmi; + u8 map_len = cea_db_payload_len(db) - 1; + u8 count; + u64 map = 0; + + if (map_len == 0) { + /* All CEA modes support ycbcr420 sampling also.*/ + hdmi->y420_cmdb_map = U64_MAX; + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; + return; + } + + /* + * This map indicates which of the existing CEA block modes + * from VDB can support YCBCR420 output too. So if bit=0 is + * set, first mode from VDB can support YCBCR420 output too. + * We will parse and keep this map, before parsing VDB itself + * to avoid going through the same block again and again. + * + * Spec is not clear about max possible size of this block. + * Clamping max bitmap block size at 8 bytes. Every byte can + * address 8 CEA modes, in this way this map can address + * 8*8 = first 64 SVDs. + */ + if (WARN_ON_ONCE(map_len > 8)) + map_len = 8; + + for (count = 0; count < map_len; count++) + map |= (u64)db[2 + count] << (8 * count); + + if (map) + info->color_formats |= DRM_COLOR_FORMAT_YCRCB420; + + hdmi->y420_cmdb_map = map; +} + static int add_cea_modes(struct drm_connector *connector, struct edid *edid) { @@ -3530,10 +3671,16 @@ add_cea_modes(struct drm_connector *connector, struct edid *edid) video = db + 1; video_len = dbl; modes += do_cea_modes(connector, video, dbl); - } - else if (cea_db_is_hdmi_vsdb(db)) { + } else if (cea_db_is_hdmi_vsdb(db)) { hdmi = db; hdmi_len = dbl; + } else if (cea_db_is_y420vdb(db)) { + const u8 *vdb420 = &db[2]; + + /* Add 4:2:0(only) modes present in EDID */ + modes += do_y420vdb_modes(connector, + vdb420, + dbl - 1); } } } @@ -4216,6 +4363,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, drm_parse_hdmi_vsdb_video(connector, db); if (cea_db_is_hdmi_forum_vsdb(db)) drm_parse_hdmi_forum_vsdb(connector, db); + if (cea_db_is_y420cmdb(db)) + drm_parse_y420cmdb_bitmap(connector, db); } } @@ -4477,6 +4626,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) num_modes += add_cea_modes(connector, edid); num_modes += add_alternate_cea_modes(connector, edid); num_modes += add_displayid_detailed_modes(connector, edid); + if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) num_modes += add_inferred_modes(connector, edid); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 26dd3eb..225e092 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -143,6 +143,17 @@ struct drm_hdmi_info { * upto 128 VICs; */ unsigned long y420_vdb_modes[BITS_TO_LONGS(128)]; + + /** + * @y420_cmdb_modes: bitmap of modes which can support ycbcr420 + * output also, along with normal HDMI outputs. There are total 107 + * VICs defined by CEA-861-F spec, so the size is 128 bits to map upto + * 128 VICs; + */ + unsigned long y420_cmdb_modes[BITS_TO_LONGS(128)]; + + /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ + u64 y420_cmdb_map; }; /** @@ -206,6 +217,7 @@ struct drm_display_info { #define DRM_COLOR_FORMAT_RGB444 (1<<0) #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) +#define DRM_COLOR_FORMAT_YCRCB420 (1<<3) /** * @color_formats: HDMI Color formats, selects between RGB and YCrCb -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 07/14] drm/edid: parse ycbcr 420 deep color information 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (5 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 08/14] drm: add helper functions for YCBCR420 handling Shashank Sharma ` (9 subsequent siblings) 16 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel; +Cc: Jose Abreu CEA-861-F spec adds ycbcr420 deep color support information in hf-vsdb block. This patch extends the existing hf-vsdb parsing function by adding parsing of ycbcr420 deep color support from the EDID and adding it into display information stored. V2: Rebase V3: Rebase V4: Moved definition of y420_dc_modes into this patch, where its used (Ville) V5: Optimize function, if(conditions) not reqd (Ville) V6: Rebase V7: Rebase Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Jose Abreu <joabreu@synopsys.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_edid.c | 12 ++++++++++++ include/drm/drm_connector.h | 3 +++ include/drm/drm_edid.h | 8 ++++++++ 3 files changed, 23 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b86afb9..6ef1f3f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4199,6 +4199,16 @@ drm_default_rgb_quant_range(const struct drm_display_mode *mode) } EXPORT_SYMBOL(drm_default_rgb_quant_range); +static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector, + const u8 *db) +{ + u8 dc_mask; + struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; + + dc_mask = db[7] & DRM_EDID_YCBCR420_DC_MASK; + hdmi->y420_dc_modes |= dc_mask; +} + static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector, const u8 *hf_vsdb) { @@ -4239,6 +4249,8 @@ static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector, scdc->scrambling.low_rates = true; } } + + drm_parse_ycbcr420_deep_color_info(connector, hf_vsdb); } static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector, diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 225e092..4bc0882 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -154,6 +154,9 @@ struct drm_hdmi_info { /** @y420_cmdb_map: bitmap of SVD index, to extraxt vcb modes */ u64 y420_cmdb_map; + + /** @y420_dc_modes: bitmap of deep color support index */ + u8 y420_dc_modes; }; /** diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 89c0062..1e1908a 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -213,6 +213,14 @@ struct detailed_timing { #define DRM_EDID_HDMI_DC_30 (1 << 4) #define DRM_EDID_HDMI_DC_Y444 (1 << 3) +/* YCBCR 420 deep color modes */ +#define DRM_EDID_YCBCR420_DC_48 (1 << 6) +#define DRM_EDID_YCBCR420_DC_36 (1 << 5) +#define DRM_EDID_YCBCR420_DC_30 (1 << 4) +#define DRM_EDID_YCBCR420_DC_MASK (DRM_EDID_YCBCR420_DC_48 | \ + DRM_EDID_YCBCR420_DC_36 | \ + DRM_EDID_YCBCR420_DC_30) + /* ELD Header Block */ #define DRM_ELD_HEADER_BLOCK_SIZE 4 -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 08/14] drm: add helper functions for YCBCR420 handling 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (6 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 07/14] drm/edid: parse ycbcr 420 deep color information Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs Shashank Sharma ` (8 subsequent siblings) 16 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel; +Cc: Jose Abreu, Daniel Vetter This patch adds helper functions for YCBCR 420 handling. These functions do: - check if a given video mode is YCBCR 420 only mode. - check if a given video mode is YCBCR 420 also mode. V2: Added YCBCR functions as helpers in DRM layer, instead of keeping it in I915 layer. V3: Added handling for YCBCR-420 only modes too. V4: EXPORT_SYMBOL(drm_find_hdmi_output_type) V5: Addressed review comments from Danvet: - %s/drm_find_hdmi_output_type/drm_display_info_hdmi_output_type - %s/drm_can_support_ycbcr_output/drm_display_supports_ycbcr_output - %s/drm_can_support_this_ycbcr_output/ drm_display_supports_this_ycbcr_output - pass drm_display_info instead of drm_connector for consistency - For drm_get_highest_quality_ycbcr_supported doc, move the variable description above, and then the function description. V6: Add only YCBCR420 helpers (Ville) V7: Addressed review comments from Ville - Remove cea_vic_valid() check. - Fix indentation. - Make input parameters to helpers, const. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jose Abreu <Jose.Abreu@synopsys.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/drm_modes.c | 58 +++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_modes.h | 6 +++++ 2 files changed, 64 insertions(+) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 35630b8..c1aec53 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1605,3 +1605,61 @@ int drm_mode_convert_umode(struct drm_display_mode *out, out: return ret; } + +/** + * drm_mode_is_420_only - if a given videomode can be only supported in YCBCR420 + * output format + * + * @connector: drm connector under action. + * @mode: video mode to be tested. + * + * Returns: + * true if the mode can be supported in YCBCR420 format + * false if not. + */ +bool drm_mode_is_420_only(const struct drm_display_info *display, + const struct drm_display_mode *mode) +{ + u8 vic = drm_match_cea_mode(mode); + + return test_bit(vic, display->hdmi.y420_vdb_modes); +} +EXPORT_SYMBOL(drm_mode_is_420_only); + +/** + * drm_mode_is_420_also - if a given videomode can be supported in YCBCR420 + * output format also (along with RGB/YCBCR444/422) + * + * @display: display under action. + * @mode: video mode to be tested. + * + * Returns: + * true if the mode can be support YCBCR420 format + * false if not. + */ +bool drm_mode_is_420_also(const struct drm_display_info *display, + const struct drm_display_mode *mode) +{ + u8 vic = drm_match_cea_mode(mode); + + return test_bit(vic, display->hdmi.y420_cmdb_modes); +} +EXPORT_SYMBOL(drm_mode_is_420_also); +/** + * drm_mode_is_420 - if a given videomode can be supported in YCBCR420 + * output format + * + * @display: display under action. + * @mode: video mode to be tested. + * + * Returns: + * true if the mode can be supported in YCBCR420 format + * false if not. + */ +bool drm_mode_is_420(const struct drm_display_info *display, + const struct drm_display_mode *mode) +{ + return drm_mode_is_420_only(display, mode) || + drm_mode_is_420_also(display, mode); +} +EXPORT_SYMBOL(drm_mode_is_420); diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index f8a1268..e9a42591 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -452,6 +452,12 @@ int drm_mode_convert_umode(struct drm_display_mode *out, const struct drm_mode_modeinfo *in); void drm_mode_probed_add(struct drm_connector *connector, struct drm_display_mode *mode); void drm_mode_debug_printmodeline(const struct drm_display_mode *mode); +bool drm_mode_is_420_only(const struct drm_display_info *display, + const struct drm_display_mode *mode); +bool drm_mode_is_420_also(const struct drm_display_info *display, + const struct drm_display_mode *mode); +bool drm_mode_is_420(const struct drm_display_info *display, + const struct drm_display_mode *mode); struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh, -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (7 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 08/14] drm: add helper functions for YCBCR420 handling Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-14 18:30 ` Ville Syrjälä 2017-07-13 15:33 ` [PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset Shashank Sharma ` (7 subsequent siblings) 16 siblings, 1 reply; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel; +Cc: Daniel Vetter This patch checks encoder level support for YCBCR420 outputs. The logic goes as simple as this: If the input mode is YCBCR420-only mode: prepare HDMI for YCBCR420 output, else continue with RGB output mode. It checks if the mode is YCBCR420 and source can support this output then it marks the ycbcr_420 output indicator into crtc state, for further staging in driver. V2: Split the patch into two, kept helper functions in DRM layer. V3: Changed the compute_config function based on new DRM API. V4: Rebase V5: Rebase V6: Check and handle YCBCR420-only modes, discard the property based approach (Ville) V7: Addressed review comments from Ville - add else case in 12BPC check. - extract ycbcr420 state inside hdmi_12bpc_possible function. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 1 + drivers/gpu/drm/i915/intel_drv.h | 3 +++ drivers/gpu/drm/i915/intel_hdmi.c | 43 +++++++++++++++++++++++++++++++++--- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2144adc..a5140e4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11945,6 +11945,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv, PIPE_CONF_CHECK_I(hdmi_scrambling); PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio); PIPE_CONF_CHECK_I(has_infoframe); + PIPE_CONF_CHECK_I(ycbcr420); PIPE_CONF_CHECK_I(has_audio); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d17a324..592243b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -780,6 +780,9 @@ struct intel_crtc_state { /* HDMI High TMDS char rate ratio */ bool hdmi_high_tmds_clock_ratio; + + /* HDMI output type */ + bool ycbcr420; }; struct intel_crtc { diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index cc0d100..eb6243c 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1312,6 +1312,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) struct drm_atomic_state *state = crtc_state->base.state; struct drm_connector_state *connector_state; struct drm_connector *connector; + bool output_ycbcr420 = crtc_state->ycbcr420; int i; if (HAS_GMCH_DISPLAY(dev_priv)) @@ -1330,8 +1331,16 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) if (connector_state->crtc != crtc_state->base.crtc) continue; - if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0) - return false; + if (output_ycbcr420) { + const struct drm_hdmi_info *hdmi = &info->hdmi; + + if (!(hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36)) + return false; + } else { + + if (!(info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36)) + return false; + } } /* Display Wa #1139 */ @@ -1342,6 +1351,25 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) return true; } +static bool +intel_hdmi_ycbcr420_config(struct drm_connector *connector, + struct intel_crtc_state *config, + int *clock_12bpc, int *clock_8bpc) +{ + + if (!connector->ycbcr_420_allowed) { + DRM_ERROR("Platform doesn't support YCBCR420 output\n"); + return false; + } + + /* YCBCR420 TMDS rate requirement is half the pixel clock */ + config->port_clock /= 2; + *clock_12bpc /= 2; + *clock_8bpc /= 2; + config->ycbcr420 = true; + return true; +} + bool intel_hdmi_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) @@ -1349,7 +1377,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; - struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc; + struct drm_connector *connector = conn_state->connector; + struct drm_scdc *scdc = &connector->display_info.hdmi.scdc; struct intel_digital_connector_state *intel_conn_state = to_intel_digital_connector_state(conn_state); int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock; @@ -1379,6 +1408,14 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, clock_12bpc *= 2; } + if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) { + if (!intel_hdmi_ycbcr420_config(connector, pipe_config, + &clock_12bpc, &clock_8bpc)) { + DRM_ERROR("Can't support YCBCR420 output\n"); + return false; + } + } + if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv)) pipe_config->has_pch_encoder = true; -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs 2017-07-13 15:33 ` [PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs Shashank Sharma @ 2017-07-14 18:30 ` Ville Syrjälä 2017-07-15 4:40 ` Sharma, Shashank 0 siblings, 1 reply; 30+ messages in thread From: Ville Syrjälä @ 2017-07-14 18:30 UTC (permalink / raw) To: Shashank Sharma; +Cc: Daniel Vetter, intel-gfx, dri-devel On Thu, Jul 13, 2017 at 09:03:15PM +0530, Shashank Sharma wrote: > This patch checks encoder level support for YCBCR420 outputs. > The logic goes as simple as this: > If the input mode is YCBCR420-only mode: prepare HDMI for > YCBCR420 output, else continue with RGB output mode. > > It checks if the mode is YCBCR420 and source can support this > output then it marks the ycbcr_420 output indicator into crtc > state, for further staging in driver. > > V2: Split the patch into two, kept helper functions in DRM layer. > V3: Changed the compute_config function based on new DRM API. > V4: Rebase > V5: Rebase > V6: Check and handle YCBCR420-only modes, discard the property > based approach (Ville) > V7: Addressed review comments from Ville > - add else case in 12BPC check. > - extract ycbcr420 state inside hdmi_12bpc_possible function. > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 1 + > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > drivers/gpu/drm/i915/intel_hdmi.c | 43 +++++++++++++++++++++++++++++++++--- > 3 files changed, 44 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2144adc..a5140e4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11945,6 +11945,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv, > PIPE_CONF_CHECK_I(hdmi_scrambling); > PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio); > PIPE_CONF_CHECK_I(has_infoframe); > + PIPE_CONF_CHECK_I(ycbcr420); > > PIPE_CONF_CHECK_I(has_audio); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index d17a324..592243b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -780,6 +780,9 @@ struct intel_crtc_state { > > /* HDMI High TMDS char rate ratio */ > bool hdmi_high_tmds_clock_ratio; > + > + /* HDMI output type */ We'll want DP too at some point. So that's going to get stale pretty soon. > + bool ycbcr420; > }; > > struct intel_crtc { > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index cc0d100..eb6243c 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1312,6 +1312,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) > struct drm_atomic_state *state = crtc_state->base.state; > struct drm_connector_state *connector_state; > struct drm_connector *connector; > + bool output_ycbcr420 = crtc_state->ycbcr420; Pointless variable. Just use 'crtc_state->ycbcr420' in the code directly. > int i; > > if (HAS_GMCH_DISPLAY(dev_priv)) > @@ -1330,8 +1331,16 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) > if (connector_state->crtc != crtc_state->base.crtc) > continue; > > - if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0) > - return false; > + if (output_ycbcr420) { > + const struct drm_hdmi_info *hdmi = &info->hdmi; > + > + if (!(hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36)) > + return false; > + } else { > + Bogus whitespace. > + if (!(info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36)) > + return false; > + } > } > > /* Display Wa #1139 */ > @@ -1342,6 +1351,25 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) > return true; > } > > +static bool > +intel_hdmi_ycbcr420_config(struct drm_connector *connector, > + struct intel_crtc_state *config, > + int *clock_12bpc, int *clock_8bpc) > +{ > + Bogus whitespace. > + if (!connector->ycbcr_420_allowed) { > + DRM_ERROR("Platform doesn't support YCBCR420 output\n"); > + return false; > + } > + > + /* YCBCR420 TMDS rate requirement is half the pixel clock */ > + config->port_clock /= 2; > + *clock_12bpc /= 2; > + *clock_8bpc /= 2; > + config->ycbcr420 = true; > + return true; > +} > + > bool intel_hdmi_compute_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config, > struct drm_connector_state *conn_state) > @@ -1349,7 +1377,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, > struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; > - struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc; > + struct drm_connector *connector = conn_state->connector; > + struct drm_scdc *scdc = &connector->display_info.hdmi.scdc; > struct intel_digital_connector_state *intel_conn_state = > to_intel_digital_connector_state(conn_state); > int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock; > @@ -1379,6 +1408,14 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, > clock_12bpc *= 2; > } > > + if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) { > + if (!intel_hdmi_ycbcr420_config(connector, pipe_config, > + &clock_12bpc, &clock_8bpc)) { > + DRM_ERROR("Can't support YCBCR420 output\n"); > + return false; > + } > + } > + > if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv)) > pipe_config->has_pch_encoder = true; > > -- > 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs 2017-07-14 18:30 ` Ville Syrjälä @ 2017-07-15 4:40 ` Sharma, Shashank 0 siblings, 0 replies; 30+ messages in thread From: Sharma, Shashank @ 2017-07-15 4:40 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx, dri-devel Regards Shashank On 7/15/2017 12:00 AM, Ville Syrjälä wrote: > On Thu, Jul 13, 2017 at 09:03:15PM +0530, Shashank Sharma wrote: >> This patch checks encoder level support for YCBCR420 outputs. >> The logic goes as simple as this: >> If the input mode is YCBCR420-only mode: prepare HDMI for >> YCBCR420 output, else continue with RGB output mode. >> >> It checks if the mode is YCBCR420 and source can support this >> output then it marks the ycbcr_420 output indicator into crtc >> state, for further staging in driver. >> >> V2: Split the patch into two, kept helper functions in DRM layer. >> V3: Changed the compute_config function based on new DRM API. >> V4: Rebase >> V5: Rebase >> V6: Check and handle YCBCR420-only modes, discard the property >> based approach (Ville) >> V7: Addressed review comments from Ville >> - add else case in 12BPC check. >> - extract ycbcr420 state inside hdmi_12bpc_possible function. >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Daniel Vetter <daniel.vetter@intel.com> >> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 1 + >> drivers/gpu/drm/i915/intel_drv.h | 3 +++ >> drivers/gpu/drm/i915/intel_hdmi.c | 43 +++++++++++++++++++++++++++++++++--- >> 3 files changed, 44 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 2144adc..a5140e4 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -11945,6 +11945,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv, >> PIPE_CONF_CHECK_I(hdmi_scrambling); >> PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio); >> PIPE_CONF_CHECK_I(has_infoframe); >> + PIPE_CONF_CHECK_I(ycbcr420); >> >> PIPE_CONF_CHECK_I(has_audio); >> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index d17a324..592243b 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -780,6 +780,9 @@ struct intel_crtc_state { >> >> /* HDMI High TMDS char rate ratio */ >> bool hdmi_high_tmds_clock_ratio; >> + >> + /* HDMI output type */ > We'll want DP too at some point. So that's going to get stale pretty > soon. Agree, will make it output format ycbcr420 >> + bool ycbcr420; >> }; >> >> struct intel_crtc { >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c >> index cc0d100..eb6243c 100644 >> --- a/drivers/gpu/drm/i915/intel_hdmi.c >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c >> @@ -1312,6 +1312,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) >> struct drm_atomic_state *state = crtc_state->base.state; >> struct drm_connector_state *connector_state; >> struct drm_connector *connector; >> + bool output_ycbcr420 = crtc_state->ycbcr420; > Pointless variable. Just use 'crtc_state->ycbcr420' in the code > directly. Ah, yes it was used at more places before we removed some of the code, during review. Sure, will use that directly. >> int i; >> >> if (HAS_GMCH_DISPLAY(dev_priv)) >> @@ -1330,8 +1331,16 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) >> if (connector_state->crtc != crtc_state->base.crtc) >> continue; >> >> - if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0) >> - return false; >> + if (output_ycbcr420) { >> + const struct drm_hdmi_info *hdmi = &info->hdmi; >> + >> + if (!(hdmi->y420_dc_modes & DRM_EDID_YCBCR420_DC_36)) >> + return false; >> + } else { >> + > Bogus whitespace. Got it. >> + if (!(info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36)) >> + return false; >> + } >> } >> >> /* Display Wa #1139 */ >> @@ -1342,6 +1351,25 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state) >> return true; >> } >> >> +static bool >> +intel_hdmi_ycbcr420_config(struct drm_connector *connector, >> + struct intel_crtc_state *config, >> + int *clock_12bpc, int *clock_8bpc) >> +{ >> + > Bogus whitespace. Got it. - Shashank >> + if (!connector->ycbcr_420_allowed) { >> + DRM_ERROR("Platform doesn't support YCBCR420 output\n"); >> + return false; >> + } >> + >> + /* YCBCR420 TMDS rate requirement is half the pixel clock */ >> + config->port_clock /= 2; >> + *clock_12bpc /= 2; >> + *clock_8bpc /= 2; >> + config->ycbcr420 = true; >> + return true; >> +} >> + >> bool intel_hdmi_compute_config(struct intel_encoder *encoder, >> struct intel_crtc_state *pipe_config, >> struct drm_connector_state *conn_state) >> @@ -1349,7 +1377,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, >> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); >> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; >> - struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc; >> + struct drm_connector *connector = conn_state->connector; >> + struct drm_scdc *scdc = &connector->display_info.hdmi.scdc; >> struct intel_digital_connector_state *intel_conn_state = >> to_intel_digital_connector_state(conn_state); >> int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock; >> @@ -1379,6 +1408,14 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, >> clock_12bpc *= 2; >> } >> >> + if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) { >> + if (!intel_hdmi_ycbcr420_config(connector, pipe_config, >> + &clock_12bpc, &clock_8bpc)) { >> + DRM_ERROR("Can't support YCBCR420 output\n"); >> + return false; >> + } >> + } >> + >> if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv)) >> pipe_config->has_pch_encoder = true; >> >> -- >> 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (8 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-14 18:30 ` Ville Syrjälä 2017-07-13 15:33 ` [PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output Shashank Sharma ` (6 subsequent siblings) 16 siblings, 1 reply; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel To get a YCBCR420 output from intel platforms, we need one scaler to scale down YCBCR444 samples to YCBCR420 samples. This patch: - Does scaler allocation for HDMI ycbcr420 outputs. - Programs PIPE_MISC register for ycbcr420 output. - Adds a new scaler user "HDMI output" to plug-into existing scaler framework. This output type is identified using bit 30 of the scaler users bitmap. V2: rebase V3: rebase V4: rebase V5: addressed review comments from Ander: - No need to check both scaler_user && hdmi_output. Check for scaler_user is enough. V6: rebase V7: Do not create a new scaler user, use existing pipe scaler user. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ander Conselvan De Oliveira <conselvan2@gmail.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 3 +++ drivers/gpu/drm/i915/intel_drv.h | 4 +++- drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++++++++ drivers/gpu/drm/i915/intel_panel.c | 3 ++- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a5140e4..d78f1c2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4624,6 +4624,9 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, */ need_scaling = src_w != dst_w || src_h != dst_h; + if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX) + need_scaling = true; + /* * Scaling/fitting not supported in IF-ID mode in GEN9+ * TODO: Interlace fetch mode doesn't support YUV420 planar formats. diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 592243b..94ea6ed 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -471,7 +471,8 @@ struct intel_crtc_scaler_state { * * If a bit is set, a user is using a scaler. * Here user can be a plane or crtc as defined below: - * bits 0-30 - plane (bit position is index from drm_plane_index) + * bits 0-29 - plane (bit position is index from drm_plane_index) + * bit 30 - hdmi output * bit 31 - crtc * * Instead of creating a new index to cover planes and crtc, using @@ -484,6 +485,7 @@ struct intel_crtc_scaler_state { * avilability. */ #define SKL_CRTC_INDEX 31 + unsigned scaler_users; /* scaler used by crtc for panel fitting purpose */ diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index eb6243c..49f4fb8 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1356,6 +1356,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector, struct intel_crtc_state *config, int *clock_12bpc, int *clock_8bpc) { + struct intel_crtc *intel_crtc = to_intel_crtc(config->base.crtc); if (!connector->ycbcr_420_allowed) { DRM_ERROR("Platform doesn't support YCBCR420 output\n"); @@ -1367,6 +1368,17 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector, *clock_12bpc /= 2; *clock_8bpc /= 2; config->ycbcr420 = true; + + /* YCBCR 420 output conversion needs a scaler */ + if (skl_update_scaler_crtc(config)) { + DRM_ERROR("Scaler allocation for output failed\n"); + return false; + } + + /* Bind this scaler to pipe */ + intel_pch_panel_fitting(intel_crtc, config, + DRM_MODE_SCALE_FULLSCREEN); + return true; } diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 96c2cbd..fd2e081 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -110,7 +110,8 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc, /* Native modes don't need fitting */ if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w && - adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) + adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h && + !pipe_config->ycbcr420) goto done; switch (fitting_mode) { -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset 2017-07-13 15:33 ` [PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset Shashank Sharma @ 2017-07-14 18:30 ` Ville Syrjälä 2017-07-15 4:41 ` Sharma, Shashank 0 siblings, 1 reply; 30+ messages in thread From: Ville Syrjälä @ 2017-07-14 18:30 UTC (permalink / raw) To: Shashank Sharma; +Cc: intel-gfx, dri-devel On Thu, Jul 13, 2017 at 09:03:16PM +0530, Shashank Sharma wrote: > To get a YCBCR420 output from intel platforms, we need one > scaler to scale down YCBCR444 samples to YCBCR420 samples. > > This patch: > - Does scaler allocation for HDMI ycbcr420 outputs. > - Programs PIPE_MISC register for ycbcr420 output. > - Adds a new scaler user "HDMI output" to plug-into existing > scaler framework. This output type is identified using bit > 30 of the scaler users bitmap. > > V2: rebase > V3: rebase > V4: rebase > V5: addressed review comments from Ander: > - No need to check both scaler_user && hdmi_output. > Check for scaler_user is enough. > V6: rebase > V7: Do not create a new scaler user, use existing pipe scaler user. > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Ander Conselvan De Oliveira <conselvan2@gmail.com> > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 3 +++ > drivers/gpu/drm/i915/intel_drv.h | 4 +++- > drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++++++++ > drivers/gpu/drm/i915/intel_panel.c | 3 ++- > 4 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index a5140e4..d78f1c2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4624,6 +4624,9 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, > */ > need_scaling = src_w != dst_w || src_h != dst_h; > > + if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX) > + need_scaling = true; > + > /* > * Scaling/fitting not supported in IF-ID mode in GEN9+ > * TODO: Interlace fetch mode doesn't support YUV420 planar formats. > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 592243b..94ea6ed 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -471,7 +471,8 @@ struct intel_crtc_scaler_state { > * > * If a bit is set, a user is using a scaler. > * Here user can be a plane or crtc as defined below: > - * bits 0-30 - plane (bit position is index from drm_plane_index) > + * bits 0-29 - plane (bit position is index from drm_plane_index) > + * bit 30 - hdmi output That's no longer true. > * bit 31 - crtc > * > * Instead of creating a new index to cover planes and crtc, using > @@ -484,6 +485,7 @@ struct intel_crtc_scaler_state { > * avilability. > */ > #define SKL_CRTC_INDEX 31 > + Bogus whitespace change. > unsigned scaler_users; > > /* scaler used by crtc for panel fitting purpose */ > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index eb6243c..49f4fb8 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1356,6 +1356,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector, > struct intel_crtc_state *config, > int *clock_12bpc, int *clock_8bpc) > { > + struct intel_crtc *intel_crtc = to_intel_crtc(config->base.crtc); > > if (!connector->ycbcr_420_allowed) { > DRM_ERROR("Platform doesn't support YCBCR420 output\n"); > @@ -1367,6 +1368,17 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector, > *clock_12bpc /= 2; > *clock_8bpc /= 2; > config->ycbcr420 = true; > + > + /* YCBCR 420 output conversion needs a scaler */ > + if (skl_update_scaler_crtc(config)) { > + DRM_ERROR("Scaler allocation for output failed\n"); DRM_DEBUG_KMS > + return false; > + } > + > + /* Bind this scaler to pipe */ Unnecesary comment. > + intel_pch_panel_fitting(intel_crtc, config, > + DRM_MODE_SCALE_FULLSCREEN); > + > return true; > } > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 96c2cbd..fd2e081 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -110,7 +110,8 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc, > > /* Native modes don't need fitting */ > if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w && > - adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) > + adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h && > + !pipe_config->ycbcr420) > goto done; > > switch (fitting_mode) { > -- > 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset 2017-07-14 18:30 ` Ville Syrjälä @ 2017-07-15 4:41 ` Sharma, Shashank 0 siblings, 0 replies; 30+ messages in thread From: Sharma, Shashank @ 2017-07-15 4:41 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, dri-devel Regards Shahank On 7/15/2017 12:00 AM, Ville Syrjälä wrote: > On Thu, Jul 13, 2017 at 09:03:16PM +0530, Shashank Sharma wrote: >> To get a YCBCR420 output from intel platforms, we need one >> scaler to scale down YCBCR444 samples to YCBCR420 samples. >> >> This patch: >> - Does scaler allocation for HDMI ycbcr420 outputs. >> - Programs PIPE_MISC register for ycbcr420 output. >> - Adds a new scaler user "HDMI output" to plug-into existing >> scaler framework. This output type is identified using bit >> 30 of the scaler users bitmap. >> >> V2: rebase >> V3: rebase >> V4: rebase >> V5: addressed review comments from Ander: >> - No need to check both scaler_user && hdmi_output. >> Check for scaler_user is enough. >> V6: rebase >> V7: Do not create a new scaler user, use existing pipe scaler user. >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Ander Conselvan De Oliveira <conselvan2@gmail.com> >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 3 +++ >> drivers/gpu/drm/i915/intel_drv.h | 4 +++- >> drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++++++++ >> drivers/gpu/drm/i915/intel_panel.c | 3 ++- >> 4 files changed, 20 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index a5140e4..d78f1c2 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -4624,6 +4624,9 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach, >> */ >> need_scaling = src_w != dst_w || src_h != dst_h; >> >> + if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX) >> + need_scaling = true; >> + >> /* >> * Scaling/fitting not supported in IF-ID mode in GEN9+ >> * TODO: Interlace fetch mode doesn't support YUV420 planar formats. >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 592243b..94ea6ed 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -471,7 +471,8 @@ struct intel_crtc_scaler_state { >> * >> * If a bit is set, a user is using a scaler. >> * Here user can be a plane or crtc as defined below: >> - * bits 0-30 - plane (bit position is index from drm_plane_index) >> + * bits 0-29 - plane (bit position is index from drm_plane_index) >> + * bit 30 - hdmi output > That's no longer true. Agree ! >> * bit 31 - crtc >> * >> * Instead of creating a new index to cover planes and crtc, using >> @@ -484,6 +485,7 @@ struct intel_crtc_scaler_state { >> * avilability. >> */ >> #define SKL_CRTC_INDEX 31 >> + > Bogus whitespace change. Got it. >> unsigned scaler_users; >> >> /* scaler used by crtc for panel fitting purpose */ >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c >> index eb6243c..49f4fb8 100644 >> --- a/drivers/gpu/drm/i915/intel_hdmi.c >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c >> @@ -1356,6 +1356,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector, >> struct intel_crtc_state *config, >> int *clock_12bpc, int *clock_8bpc) >> { >> + struct intel_crtc *intel_crtc = to_intel_crtc(config->base.crtc); >> >> if (!connector->ycbcr_420_allowed) { >> DRM_ERROR("Platform doesn't support YCBCR420 output\n"); >> @@ -1367,6 +1368,17 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector, >> *clock_12bpc /= 2; >> *clock_8bpc /= 2; >> config->ycbcr420 = true; >> + >> + /* YCBCR 420 output conversion needs a scaler */ >> + if (skl_update_scaler_crtc(config)) { >> + DRM_ERROR("Scaler allocation for output failed\n"); > DRM_DEBUG_KMS Ok, >> + return false; >> + } >> + >> + /* Bind this scaler to pipe */ > Unnecesary comment. Ok, Shashank >> + intel_pch_panel_fitting(intel_crtc, config, >> + DRM_MODE_SCALE_FULLSCREEN); >> + >> return true; >> } >> >> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c >> index 96c2cbd..fd2e081 100644 >> --- a/drivers/gpu/drm/i915/intel_panel.c >> +++ b/drivers/gpu/drm/i915/intel_panel.c >> @@ -110,7 +110,8 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc, >> >> /* Native modes don't need fitting */ >> if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w && >> - adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h) >> + adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h && >> + !pipe_config->ycbcr420) >> goto done; >> >> switch (fitting_mode) { >> -- >> 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (9 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-14 18:33 ` Ville Syrjälä 2017-07-13 15:33 ` [PATCH v2 12/14] drm/i915: prepare csc unit " Shashank Sharma ` (5 subsequent siblings) 16 siblings, 1 reply; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel; +Cc: Daniel Vetter To get HDMI YCBCR420 output, the PIPEMISC register should be programmed to: - Generate YCBCR output (bit 11) - In case of YCBCR420 outputs, it should be programmed in full blend mode to use the scaler in 5x3 ratio (bits 26 and 27) This patch: - Adds definition of these bits. - Programs PIPEMISC for YCBCR420 outputs. V2: rebase V3: rebase V4: rebase V5: added r-b from Ander V6: Handle only YCBCR420 outputs (ville) V7: rebase Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_display.c | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c712d01..e5020d6 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5227,6 +5227,9 @@ enum { #define _PIPE_MISC_A 0x70030 #define _PIPE_MISC_B 0x71030 +#define PIPEMISC_YCBCR420_ENABLE (1<<27) +#define PIPEMISC_YCBCR420_MODE_BLEND (1<<26) +#define PIPEMISC_OUTPUT_YCBCR (1<<11) #define PIPEMISC_DITHER_BPC_MASK (7<<5) #define PIPEMISC_DITHER_8_BPC (0<<5) #define PIPEMISC_DITHER_10_BPC (1<<5) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d78f1c2..1a23ec0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8076,6 +8076,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc_state *config = intel_crtc->config; if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) { u32 val = 0; @@ -8101,6 +8102,12 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc) if (intel_crtc->config->dither) val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP; + if (config->ycbcr420) { + val |= PIPEMISC_OUTPUT_YCBCR | + PIPEMISC_YCBCR420_ENABLE | + PIPEMISC_YCBCR420_MODE_BLEND; + } + I915_WRITE(PIPEMISC(intel_crtc->pipe), val); } } -- 2.7.4 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output 2017-07-13 15:33 ` [PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output Shashank Sharma @ 2017-07-14 18:33 ` Ville Syrjälä 2017-07-15 4:45 ` Sharma, Shashank 0 siblings, 1 reply; 30+ messages in thread From: Ville Syrjälä @ 2017-07-14 18:33 UTC (permalink / raw) To: Shashank Sharma; +Cc: Daniel Vetter, intel-gfx, dri-devel On Thu, Jul 13, 2017 at 09:03:17PM +0530, Shashank Sharma wrote: > To get HDMI YCBCR420 output, the PIPEMISC register should be > programmed to: > - Generate YCBCR output (bit 11) > - In case of YCBCR420 outputs, it should be programmed in full > blend mode to use the scaler in 5x3 ratio (bits 26 and 27) > > This patch: > - Adds definition of these bits. > - Programs PIPEMISC for YCBCR420 outputs. > > V2: rebase > V3: rebase > V4: rebase > V5: added r-b from Ander > V6: Handle only YCBCR420 outputs (ville) > V7: rebase > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > > Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > drivers/gpu/drm/i915/intel_display.c | 7 +++++++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c712d01..e5020d6 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5227,6 +5227,9 @@ enum { > > #define _PIPE_MISC_A 0x70030 > #define _PIPE_MISC_B 0x71030 > +#define PIPEMISC_YCBCR420_ENABLE (1<<27) > +#define PIPEMISC_YCBCR420_MODE_BLEND (1<<26) > +#define PIPEMISC_OUTPUT_YCBCR (1<<11) Please rename to match spec. So something like: PIPEMISC_YUV420_ENABLE (1<<27) PIPEMISC_YUV420_MODE_FULL_BLEND (1<<26) PIPEMISC_OUTPUT_COLORSPACE_YUV (1<<11) > #define PIPEMISC_DITHER_BPC_MASK (7<<5) > #define PIPEMISC_DITHER_8_BPC (0<<5) > #define PIPEMISC_DITHER_10_BPC (1<<5) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index d78f1c2..1a23ec0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8076,6 +8076,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc) > { > struct drm_i915_private *dev_priv = to_i915(crtc->dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc_state *config = intel_crtc->config; > > if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) { > u32 val = 0; > @@ -8101,6 +8102,12 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc) > if (intel_crtc->config->dither) > val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP; > > + if (config->ycbcr420) { > + val |= PIPEMISC_OUTPUT_YCBCR | > + PIPEMISC_YCBCR420_ENABLE | > + PIPEMISC_YCBCR420_MODE_BLEND; > + } I think we'll want two flags. One to specify whether we're outputting YCbCr and the other to indicate whether we need the 4:2:0 subsamling. So maybe something like bool ycbcr; bool ycbcr420; We also need state readout for this stuff. With those two flags I think we can do something like: if (IS_BDW || GEN >= 9) { tmp = READ(PIPEMISC); crtc_state->ycbcr = tmp & OUTPUT_YUV; if (IS_GLK || GEN >= 10) crtc_state->ycbcr420 = tmp & YUV420_ENABLE; } The other missing readout thing is adjustment of the clock. ddi_dotclock_get() will need to double the dotclock when we're outputting ycbcr420. Pls also add something along the lines of DRM_DEBUG_KMS("ycbcr: %i, ycbcr420: %i\n", ...); to intel_dump_pipe_config() so that we can actually tell when we're outputting YCbCr and 4:2:0. > + > I915_WRITE(PIPEMISC(intel_crtc->pipe), val); > } > } > -- > 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output 2017-07-14 18:33 ` Ville Syrjälä @ 2017-07-15 4:45 ` Sharma, Shashank 0 siblings, 0 replies; 30+ messages in thread From: Sharma, Shashank @ 2017-07-15 4:45 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx, dri-devel Regards Shashank On 7/15/2017 12:03 AM, Ville Syrjälä wrote: > On Thu, Jul 13, 2017 at 09:03:17PM +0530, Shashank Sharma wrote: >> To get HDMI YCBCR420 output, the PIPEMISC register should be >> programmed to: >> - Generate YCBCR output (bit 11) >> - In case of YCBCR420 outputs, it should be programmed in full >> blend mode to use the scaler in 5x3 ratio (bits 26 and 27) >> >> This patch: >> - Adds definition of these bits. >> - Programs PIPEMISC for YCBCR420 outputs. >> >> V2: rebase >> V3: rebase >> V4: rebase >> V5: added r-b from Ander >> V6: Handle only YCBCR420 outputs (ville) >> V7: rebase >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> >> Cc: Daniel Vetter <daniel.vetter@intel.com> >> >> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 3 +++ >> drivers/gpu/drm/i915/intel_display.c | 7 +++++++ >> 2 files changed, 10 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index c712d01..e5020d6 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -5227,6 +5227,9 @@ enum { >> >> #define _PIPE_MISC_A 0x70030 >> #define _PIPE_MISC_B 0x71030 >> +#define PIPEMISC_YCBCR420_ENABLE (1<<27) >> +#define PIPEMISC_YCBCR420_MODE_BLEND (1<<26) >> +#define PIPEMISC_OUTPUT_YCBCR (1<<11) > Please rename to match spec. So something like: > PIPEMISC_YUV420_ENABLE (1<<27) > PIPEMISC_YUV420_MODE_FULL_BLEND (1<<26) > PIPEMISC_OUTPUT_COLORSPACE_YUV (1<<11) got it. >> #define PIPEMISC_DITHER_BPC_MASK (7<<5) >> #define PIPEMISC_DITHER_8_BPC (0<<5) >> #define PIPEMISC_DITHER_10_BPC (1<<5) >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index d78f1c2..1a23ec0 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -8076,6 +8076,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc) >> { >> struct drm_i915_private *dev_priv = to_i915(crtc->dev); >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> + struct intel_crtc_state *config = intel_crtc->config; >> >> if (IS_BROADWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 9) { >> u32 val = 0; >> @@ -8101,6 +8102,12 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc) >> if (intel_crtc->config->dither) >> val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP; >> >> + if (config->ycbcr420) { >> + val |= PIPEMISC_OUTPUT_YCBCR | >> + PIPEMISC_YCBCR420_ENABLE | >> + PIPEMISC_YCBCR420_MODE_BLEND; >> + } > I think we'll want two flags. One to specify whether we're outputting > YCbCr and the other to indicate whether we need the 4:2:0 subsamling. > So maybe something like > bool ycbcr; > bool ycbcr420; This would have been true if we were support YCBCR444/422/420 all, but the recent patch series only supports 420, so if its ycbcr its ycbcr420. We might be able to add preference of a 420_also mode in case of RGB Vs YCBCR420, by adding another property. > We also need state readout for this stuff. With those two flags I think > we can do something like: > > if (IS_BDW || GEN >= 9) { > tmp = READ(PIPEMISC); > > crtc_state->ycbcr = tmp & OUTPUT_YUV; We dont support YCBCR (apart from 420) anymore, as there is no HDMI output property. So this doesn't look required. > > if (IS_GLK || GEN >= 10) > crtc_state->ycbcr420 = tmp & YUV420_ENABLE; Yes, this is a great suggestion, and I believe I was missing this. Thanks ! > } > > The other missing readout thing is adjustment of the clock. > ddi_dotclock_get() will need to double the dotclock when we're > outputting ycbcr420. > > Pls also add something along the lines of > DRM_DEBUG_KMS("ycbcr: %i, ycbcr420: %i\n", ...); > to intel_dump_pipe_config() so that we can actually tell when we're > outputting YCbCr and 4:2:0. Thanks, this is also what I was looking for. - Shashank > >> + >> I915_WRITE(PIPEMISC(intel_crtc->pipe), val); >> } >> } >> -- >> 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 12/14] drm/i915: prepare csc unit for YCBCR420 output 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (10 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-14 18:36 ` Ville Syrjälä 2017-07-13 15:33 ` [PATCH v2 13/14] drm/i915: set colorspace for YCBCR420 outputs Shashank Sharma ` (4 subsequent siblings) 16 siblings, 1 reply; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel; +Cc: Daniel Vetter To support ycbcr output, we need a pipe CSC block to do RGB->YCBCR conversion. Current Intel platforms have only one pipe CSC unit, so we can either do color correction using it, or we can perform RGB->YCBCR conversion. This function adds a csc handler, which uses recommended bspec values to perform RGB->YCBCR conversion (target color space BT709) V2: Rebase V3: Rebase V4: Rebase V5: Addressed review comments from Ander - Remove extra line added in the patch - Add the spec details in the commit message - Combine two if(cond) while calling intel_crtc_compute_config V6: Handle YCBCR420 outputs only (Ville) V7: Addressed review comments from Ville: - Add description about target colorspace - Remove the comments from CSC function - DRM_DEBUG->DEBUG_KMS for atomic failure due to CSC unit busy - Remove unnecessary debug message about YCBCR420 possibe Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/i915/intel_color.c | 46 +++++++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index f85d575..8698691 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -41,6 +41,22 @@ #define LEGACY_LUT_LENGTH (sizeof(struct drm_color_lut) * 256) +/* Post offset values for RGB->YCBCR conversion */ +#define POSTOFF_RGB_TO_YUV_HI 0x800 +#define POSTOFF_RGB_TO_YUV_ME 0x100 +#define POSTOFF_RGB_TO_YUV_LO 0x800 + +/* + * These values are direct register values specified in the Bspec, + * for RGB->YUV conversion matrix (colorspace BT709) + */ +#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8 +#define CSC_RGB_TO_YUV_BU 0x37e80000 +#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0 +#define CSC_RGB_TO_YUV_BY 0xb5280000 +#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 +#define CSC_RGB_TO_YUV_BV 0x1e080000 + /* * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point * format). This macro takes the coefficient we want transformed and the @@ -91,6 +107,31 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input) } } +void i9xx_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc) +{ + int pipe = intel_crtc->pipe; + struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); + + + I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); + I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); + I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0); + + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), CSC_RGB_TO_YUV_RU_GU); + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), CSC_RGB_TO_YUV_BU); + + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), CSC_RGB_TO_YUV_RY_GY); + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), CSC_RGB_TO_YUV_BY); + + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), CSC_RGB_TO_YUV_RV_GV); + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), CSC_RGB_TO_YUV_BV); + + I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI); + I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME); + I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO); + I915_WRITE(PIPE_CSC_MODE(pipe), 0); +} + /* Set up the pipe CSC unit. */ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) { @@ -101,7 +142,10 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) uint16_t coeffs[9] = { 0, }; struct intel_crtc_state *intel_crtc_state = to_intel_crtc_state(crtc_state); - if (crtc_state->ctm) { + if (intel_crtc_state->ycbcr420) { + i9xx_load_ycbcr_conversion_matrix(intel_crtc); + return; + } else if (crtc_state->ctm) { struct drm_color_ctm *ctm = (struct drm_color_ctm *)crtc_state->ctm->data; uint64_t input[9] = { 0, }; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1a23ec0..9c6a1ed 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6283,6 +6283,21 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, return -EINVAL; } + /* YCBCR420 feasibility check */ + if (pipe_config->ycbcr420) { + struct drm_crtc_state *drm_state = &pipe_config->base; + + /* + * There is only one pipe CSC unit per pipe, and we need that + * for output conversion from RGB->YCBCR. So if CTM is already + * applied we can't support YCBCR420 output. + */ + if (drm_state->ctm) { + DRM_DEBUG_KMS("YCBCR420 and CTM is not possible\n"); + return -EINVAL; + } + } + /* * Pipe horizontal size must be even in: * - DVO ganged mode -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 12/14] drm/i915: prepare csc unit for YCBCR420 output 2017-07-13 15:33 ` [PATCH v2 12/14] drm/i915: prepare csc unit " Shashank Sharma @ 2017-07-14 18:36 ` Ville Syrjälä 2017-07-15 4:46 ` Sharma, Shashank 0 siblings, 1 reply; 30+ messages in thread From: Ville Syrjälä @ 2017-07-14 18:36 UTC (permalink / raw) To: Shashank Sharma; +Cc: Daniel Vetter, intel-gfx, dri-devel On Thu, Jul 13, 2017 at 09:03:18PM +0530, Shashank Sharma wrote: > To support ycbcr output, we need a pipe CSC block to do > RGB->YCBCR conversion. > > Current Intel platforms have only one pipe CSC unit, so > we can either do color correction using it, or we can perform > RGB->YCBCR conversion. > > This function adds a csc handler, which uses recommended bspec > values to perform RGB->YCBCR conversion (target color space BT709) > > V2: Rebase > V3: Rebase > V4: Rebase > V5: Addressed review comments from Ander > - Remove extra line added in the patch > - Add the spec details in the commit message > - Combine two if(cond) while calling intel_crtc_compute_config > V6: Handle YCBCR420 outputs only (Ville) > V7: Addressed review comments from Ville: > - Add description about target colorspace > - Remove the comments from CSC function > - DRM_DEBUG->DEBUG_KMS for atomic failure due to CSC unit busy > - Remove unnecessary debug message about YCBCR420 possibe > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/i915/intel_color.c | 46 +++++++++++++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++ > 2 files changed, 60 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index f85d575..8698691 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -41,6 +41,22 @@ > > #define LEGACY_LUT_LENGTH (sizeof(struct drm_color_lut) * 256) > > +/* Post offset values for RGB->YCBCR conversion */ > +#define POSTOFF_RGB_TO_YUV_HI 0x800 > +#define POSTOFF_RGB_TO_YUV_ME 0x100 > +#define POSTOFF_RGB_TO_YUV_LO 0x800 > + > +/* > + * These values are direct register values specified in the Bspec, > + * for RGB->YUV conversion matrix (colorspace BT709) > + */ > +#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8 > +#define CSC_RGB_TO_YUV_BU 0x37e80000 > +#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0 > +#define CSC_RGB_TO_YUV_BY 0xb5280000 > +#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 > +#define CSC_RGB_TO_YUV_BV 0x1e080000 > + > /* > * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point > * format). This macro takes the coefficient we want transformed and the > @@ -91,6 +107,31 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input) > } > } > > +void i9xx_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc) > +{ > + int pipe = intel_crtc->pipe; > + struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); > + > + > + I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); > + I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); > + I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0); > + > + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), CSC_RGB_TO_YUV_RU_GU); > + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), CSC_RGB_TO_YUV_BU); > + > + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), CSC_RGB_TO_YUV_RY_GY); > + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), CSC_RGB_TO_YUV_BY); > + > + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), CSC_RGB_TO_YUV_RV_GV); > + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), CSC_RGB_TO_YUV_BV); > + > + I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI); > + I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME); > + I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO); > + I915_WRITE(PIPE_CSC_MODE(pipe), 0); > +} > + > /* Set up the pipe CSC unit. */ > static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) > { > @@ -101,7 +142,10 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) > uint16_t coeffs[9] = { 0, }; > struct intel_crtc_state *intel_crtc_state = to_intel_crtc_state(crtc_state); > > - if (crtc_state->ctm) { > + if (intel_crtc_state->ycbcr420) { > + i9xx_load_ycbcr_conversion_matrix(intel_crtc); > + return; > + } else if (crtc_state->ctm) { > struct drm_color_ctm *ctm = > (struct drm_color_ctm *)crtc_state->ctm->data; > uint64_t input[9] = { 0, }; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 1a23ec0..9c6a1ed 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6283,6 +6283,21 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, > return -EINVAL; > } > > + /* YCBCR420 feasibility check */ Not a useful comment. > + if (pipe_config->ycbcr420) { > + struct drm_crtc_state *drm_state = &pipe_config->base; Having aliasing pointer for the same thing is just annoying. We do have quite a lot of that going around, but IMO we need to clean that all up at some point. So pls just do this instead: if (pipe_config->ycbcr420 && pipe_config->base.ctm) { ... } > + > + /* > + * There is only one pipe CSC unit per pipe, and we need that > + * for output conversion from RGB->YCBCR. So if CTM is already > + * applied we can't support YCBCR420 output. > + */ > + if (drm_state->ctm) { > + DRM_DEBUG_KMS("YCBCR420 and CTM is not possible\n"); > + return -EINVAL; > + } > + } > + > /* > * Pipe horizontal size must be even in: > * - DVO ganged mode > -- > 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 12/14] drm/i915: prepare csc unit for YCBCR420 output 2017-07-14 18:36 ` Ville Syrjälä @ 2017-07-15 4:46 ` Sharma, Shashank 0 siblings, 0 replies; 30+ messages in thread From: Sharma, Shashank @ 2017-07-15 4:46 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx, dri-devel Regards Shashank On 7/15/2017 12:06 AM, Ville Syrjälä wrote: > On Thu, Jul 13, 2017 at 09:03:18PM +0530, Shashank Sharma wrote: >> To support ycbcr output, we need a pipe CSC block to do >> RGB->YCBCR conversion. >> >> Current Intel platforms have only one pipe CSC unit, so >> we can either do color correction using it, or we can perform >> RGB->YCBCR conversion. >> >> This function adds a csc handler, which uses recommended bspec >> values to perform RGB->YCBCR conversion (target color space BT709) >> >> V2: Rebase >> V3: Rebase >> V4: Rebase >> V5: Addressed review comments from Ander >> - Remove extra line added in the patch >> - Add the spec details in the commit message >> - Combine two if(cond) while calling intel_crtc_compute_config >> V6: Handle YCBCR420 outputs only (Ville) >> V7: Addressed review comments from Ville: >> - Add description about target colorspace >> - Remove the comments from CSC function >> - DRM_DEBUG->DEBUG_KMS for atomic failure due to CSC unit busy >> - Remove unnecessary debug message about YCBCR420 possibe >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Daniel Vetter <daniel.vetter@intel.com> >> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/i915/intel_color.c | 46 +++++++++++++++++++++++++++++++++++- >> drivers/gpu/drm/i915/intel_display.c | 15 ++++++++++++ >> 2 files changed, 60 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c >> index f85d575..8698691 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -41,6 +41,22 @@ >> >> #define LEGACY_LUT_LENGTH (sizeof(struct drm_color_lut) * 256) >> >> +/* Post offset values for RGB->YCBCR conversion */ >> +#define POSTOFF_RGB_TO_YUV_HI 0x800 >> +#define POSTOFF_RGB_TO_YUV_ME 0x100 >> +#define POSTOFF_RGB_TO_YUV_LO 0x800 >> + >> +/* >> + * These values are direct register values specified in the Bspec, >> + * for RGB->YUV conversion matrix (colorspace BT709) >> + */ >> +#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8 >> +#define CSC_RGB_TO_YUV_BU 0x37e80000 >> +#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0 >> +#define CSC_RGB_TO_YUV_BY 0xb5280000 >> +#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8 >> +#define CSC_RGB_TO_YUV_BV 0x1e080000 >> + >> /* >> * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point >> * format). This macro takes the coefficient we want transformed and the >> @@ -91,6 +107,31 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input) >> } >> } >> >> +void i9xx_load_ycbcr_conversion_matrix(struct intel_crtc *intel_crtc) >> +{ >> + int pipe = intel_crtc->pipe; >> + struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev); >> + >> + >> + I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); >> + I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); >> + I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0); >> + >> + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), CSC_RGB_TO_YUV_RU_GU); >> + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), CSC_RGB_TO_YUV_BU); >> + >> + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), CSC_RGB_TO_YUV_RY_GY); >> + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), CSC_RGB_TO_YUV_BY); >> + >> + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), CSC_RGB_TO_YUV_RV_GV); >> + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), CSC_RGB_TO_YUV_BV); >> + >> + I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI); >> + I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME); >> + I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO); >> + I915_WRITE(PIPE_CSC_MODE(pipe), 0); >> +} >> + >> /* Set up the pipe CSC unit. */ >> static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) >> { >> @@ -101,7 +142,10 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state) >> uint16_t coeffs[9] = { 0, }; >> struct intel_crtc_state *intel_crtc_state = to_intel_crtc_state(crtc_state); >> >> - if (crtc_state->ctm) { >> + if (intel_crtc_state->ycbcr420) { >> + i9xx_load_ycbcr_conversion_matrix(intel_crtc); >> + return; >> + } else if (crtc_state->ctm) { >> struct drm_color_ctm *ctm = >> (struct drm_color_ctm *)crtc_state->ctm->data; >> uint64_t input[9] = { 0, }; >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 1a23ec0..9c6a1ed 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -6283,6 +6283,21 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, >> return -EINVAL; >> } >> >> + /* YCBCR420 feasibility check */ > Not a useful comment. Got it. >> + if (pipe_config->ycbcr420) { >> + struct drm_crtc_state *drm_state = &pipe_config->base; > Having aliasing pointer for the same thing is just annoying. > We do have quite a lot of that going around, but IMO we need > to clean that all up at some point. > > So pls just do this instead: > > if (pipe_config->ycbcr420 && pipe_config->base.ctm) { > ... > } Most of the time its done to cover for 80 char limit, and to make code look good. but let me see if I can accommodate this one. - Shashank > >> + >> + /* >> + * There is only one pipe CSC unit per pipe, and we need that >> + * for output conversion from RGB->YCBCR. So if CTM is already >> + * applied we can't support YCBCR420 output. >> + */ >> + if (drm_state->ctm) { >> + DRM_DEBUG_KMS("YCBCR420 and CTM is not possible\n"); >> + return -EINVAL; >> + } >> + } >> + >> /* >> * Pipe horizontal size must be even in: >> * - DVO ganged mode >> -- >> 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 13/14] drm/i915: set colorspace for YCBCR420 outputs 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (11 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 12/14] drm/i915: prepare csc unit " Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 14/14] drm/i915/glk: set HDMI 2.0 identifier Shashank Sharma ` (3 subsequent siblings) 16 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel When output colorspace is YCBCR420, we have to load the corresponding colorspace in AVI infoframe. This patch fills the colorspace of AVI infoframe as per the output mode. V2: Rebase V3: Rebase V4: Rebase V5: Added r-b from Ander V6: Checking RGB/YCBCR420 output only (Ville) V7: Add colorspace info in driver(not drm layer) (Ville) Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/i915/intel_hdmi.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 49f4fb8..412b6e3 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -472,12 +472,18 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder, return; } + if (crtc_state->ycbcr420) + frame.avi.colorspace = HDMI_COLORSPACE_YUV420; + else + frame.avi.colorspace = HDMI_COLORSPACE_RGB; + drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode, crtc_state->limited_color_range ? HDMI_QUANTIZATION_RANGE_LIMITED : HDMI_QUANTIZATION_RANGE_FULL, intel_hdmi->rgb_quant_range_selectable); + /* TODO: handle pixel repetition for YCBCR420 outputs */ intel_write_infoframe(encoder, crtc_state, &frame); } -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 14/14] drm/i915/glk: set HDMI 2.0 identifier 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (12 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 13/14] drm/i915: set colorspace for YCBCR420 outputs Shashank Sharma @ 2017-07-13 15:33 ` Shashank Sharma 2017-07-13 15:50 ` ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev2) Patchwork ` (2 subsequent siblings) 16 siblings, 0 replies; 30+ messages in thread From: Shashank Sharma @ 2017-07-13 15:33 UTC (permalink / raw) To: intel-gfx, dri-devel This patch sets the is_hdmi2_src identifier in drm connector for GLK platform. GLK contains a native HDMI 2.0 controller. This identifier will help the EDID handling functions to save lot of work which is specific to HDMI 2.0 sources. V3: Added this patch V4: Rebase V4: Rebase V5: Added r-b from Ander V6: Rebase V7: Rebase Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/i915/intel_hdmi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 412b6e3..bd08ae4 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1917,6 +1917,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, connector->doublescan_allowed = 0; connector->stereo_allowed = 1; + if (IS_GEMINILAKE(dev_priv)) + connector->ycbcr_420_allowed = true; + intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port); switch (port) { -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 30+ messages in thread
* ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev2) 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (13 preceding siblings ...) 2017-07-13 15:33 ` [PATCH v2 14/14] drm/i915/glk: set HDMI 2.0 identifier Shashank Sharma @ 2017-07-13 15:50 ` Patchwork 2017-07-14 10:57 ` ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev3) Patchwork 2017-07-14 19:02 ` [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Ville Syrjälä 16 siblings, 0 replies; 30+ messages in thread From: Patchwork @ 2017-07-13 15:50 UTC (permalink / raw) To: Shashank Sharma; +Cc: intel-gfx == Series Details == Series: YCBCR 4:2:0 handling in DRM layer (rev2) URL : https://patchwork.freedesktop.org/series/26972/ State : success == Summary == Series 26972v2 YCBCR 4:2:0 handling in DRM layer https://patchwork.freedesktop.org/api/1.0/series/26972/revisions/2/mbox/ fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:437s fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:424s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:358s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:538s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:512s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:490s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:485s fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:592s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:434s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:411s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:415s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:498s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:469s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:463s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:566s fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:575s fi-pnv-d510 total:279 pass:222 dwarn:2 dfail:0 fail:0 skip:55 time:563s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:454s fi-skl-6700hq total:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:580s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:474s fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:482s fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:430s fi-skl-x1585l total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:486s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:535s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:400s 8ad9e19aafea47c272163c2cbf554e06ff7f9857 drm-tip: 2017y-07m-11d-19h-08m-20s UTC integration manifest 706180a drm/i915/glk: set HDMI 2.0 identifier 23c1aaa drm/i915: set colorspace for YCBCR420 outputs f1885be drm/i915: prepare csc unit for YCBCR420 output d550d0a drm/i915: prepare pipe for YCBCR420 output 0d4052d drm/i915: prepare scaler for YCBCR420 modeset 230c4ad drm/i915: add config function for YCBCR420 outputs 93fe7db drm: add helper functions for YCBCR420 handling 426d8ac drm/edid: parse ycbcr 420 deep color information 1fd56bd drm/edid: parse YCBCR420 videomodes from EDID cb40a11 drm: add helper to validate YCBCR420 modes 9bff1cf drm/edid: cleanup patch for CEA extended-tag macro a2d60dc drm/edid: parse sink information before CEA blocks fd87ef7 drm/edid: complete CEA modedb(VIC 1-107) 510e1eb drm: handle HDMI 2.0 VICs in AVI info-frames == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5184/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 30+ messages in thread
* ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev3) 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (14 preceding siblings ...) 2017-07-13 15:50 ` ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev2) Patchwork @ 2017-07-14 10:57 ` Patchwork 2017-07-14 19:02 ` [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Ville Syrjälä 16 siblings, 0 replies; 30+ messages in thread From: Patchwork @ 2017-07-14 10:57 UTC (permalink / raw) To: Shashank Sharma; +Cc: intel-gfx == Series Details == Series: YCBCR 4:2:0 handling in DRM layer (rev3) URL : https://patchwork.freedesktop.org/series/26972/ State : success == Summary == Series 26972v3 YCBCR 4:2:0 handling in DRM layer https://patchwork.freedesktop.org/api/1.0/series/26972/revisions/3/mbox/ Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> FAIL (fi-skl-6770hq) fdo#99739 Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: pass -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1 fdo#99739 https://bugs.freedesktop.org/show_bug.cgi?id=99739 fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597 fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:441s fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:431s fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:351s fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:529s fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:505s fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:491s fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:481s fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:597s fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:438s fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:409s fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:493s fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:474s fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:458s fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:574s fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:583s fi-pnv-d510 total:279 pass:221 dwarn:3 dfail:0 fail:0 skip:55 time:562s fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:456s fi-skl-6700hq total:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:585s fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:468s fi-skl-6770hq total:279 pass:268 dwarn:0 dfail:0 fail:1 skip:10 time:467s fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:432s fi-skl-x1585l total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:472s fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:543s fi-snb-2600 total:279 pass:250 dwarn:0 dfail:0 fail:0 skip:29 time:399s 5182881eef4843638e993d15f93a8dbb8c89e429 drm-tip: 2017y-07m-13d-18h-13m-28s UTC integration manifest 219b18a drm/i915/glk: set HDMI 2.0 identifier 062c202 drm/i915: set colorspace for YCBCR420 outputs 4f53995 drm/i915: prepare csc unit for YCBCR420 output bc10a02 drm/i915: prepare pipe for YCBCR420 output eb22487 drm/i915: prepare scaler for YCBCR420 modeset 5a003aa drm/i915: add config function for YCBCR420 outputs d646c9b drm: add helper functions for YCBCR420 handling 3834ed9 drm/edid: parse ycbcr 420 deep color information 7434121 drm/edid: parse YCBCR420 videomodes from EDID 80d0b03 drm: add helper to validate YCBCR420 modes 7d0cad6 drm/edid: cleanup patch for CEA extended-tag macro 407172b drm/edid: parse sink information before CEA blocks 0d3a770 drm/edid: complete CEA modedb(VIC 1-107) f29dc09 drm: handle HDMI 2.0 VICs in AVI info-frames == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5188/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma ` (15 preceding siblings ...) 2017-07-14 10:57 ` ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev3) Patchwork @ 2017-07-14 19:02 ` Ville Syrjälä 2017-07-15 5:03 ` Sharma, Shashank 16 siblings, 1 reply; 30+ messages in thread From: Ville Syrjälä @ 2017-07-14 19:02 UTC (permalink / raw) To: Shashank Sharma; +Cc: intel-gfx, dri-devel On Thu, Jul 13, 2017 at 09:03:06PM +0530, Shashank Sharma wrote: > Following YCBCR 4:4:4 and 4:2:2, YCBCR 4:2:0 is a new output format, > which is currently supported on HDMI 2.0 sources/sinks. Due to lower > chroma sub-sampling rate, YCBCR 4:2:0 can drive the video modes at half > the pixel clock than YCBCR 4:4:4 or RGB 8:8:8 outputs. For example, a CEA > 4K@60, RGB 8:8:8 mode needs a clock of appx 594Mhz, but it can be driven at > 297Mhz using YCBCR 4:2:0 output. > > Of course, the lower rate of chroma subsampling, causes the quality of YCBCR > 4:2:0 to be lower than YCBCR 4:4:4 or RGB 8:8:8. > > This patch series adds support for YCBCR 4:2:0 output in DRM layer. > > - First 2 patches, complete the CEA mode-db in drm driver, by adding > new 4k modes. Current CEA mode-db contains 64 modes only (VIC 1-64), > whereas CEA-861-F defined VICs up to 107, including 4k modes, from VIC > range 93-107. First patch makes sure that inclusion of these modes doesn't > break existing HDMI 1.4 monitors, across various drivers. > > - Next 5 patches focus on parsing new YCBCR 4:2:0 EDID blocks, and adding > YCBCR 4:2:0 modes in connector. They also contain a prune function, which > will cleanup the YCBCR 4:2:0 modes from list, if the connector doesn't > declare them supported. > > - Next 2 patches add helper functions for identifing YCBCR 4:2:0 modes and > setup the color space in AVI infoframes. > > - Next 6 patches add code for I915 layer handling of YCBCR 4:2:0 output. > > This patch series was initially published as a complete framework to handle > all YCBCR outputs (4:4:4, 4:2:2, 4:2:0), but based on the code reviews, now > its been published as YCBCR 4:2:0 handling series only. > > The previous discussion and reviews can be found here: > V5: https://patchwork.freedesktop.org/series/26815/ > V1-V4: https://patchwork.freedesktop.org/series/22683/ > > Now re-publishing this patch series as YCBCR 4:2:0 handling series here. > V2: Addressed review comments from Ville > This series dropped one patch in V2(patch 8 from V1), so 14 patches in V2 > > This series has been tested with drm-tip code with following setup: > Source: Intel Geminilake device. > Sink: ACER S277HK HDMI 2.0 monitor. > Protocol testing: Astro VA-1844A HDMI analyzer. > > Shashank Sharma (14): > drm: handle HDMI 2.0 VICs in AVI info-frames > drm/edid: complete CEA modedb(VIC 1-107) > drm/edid: parse sink information before CEA blocks > drm/edid: cleanup patch for CEA extended-tag macro > drm: add helper to validate YCBCR420 modes > drm/edid: parse YCBCR420 videomodes from EDID > drm/edid: parse ycbcr 420 deep color information > drm: add helper functions for YCBCR420 handling I just finished pushing the core bits into drm-misc-next. But I'm not super happy how that went because there were still quite a few trivial checkpatch warnings and indentation issues I had to fix up. All of that should have been dealt with before even submitting the patches to the list. Review bandwidth is a scarce resource so we don't want to squander it on this sort of stuff. Another issue I ran into when I tried to actually run the code. As soon as I loaded the driver a warning and the accompanying backtrace from the state checker flew past my terminal. That too should have been caught before even sending the patches to the list. So I think the next time someone asks me to review something I will start by asking these basic questions: - Did you configure your editor so that it automagically formats code correctly? - Did you check for new compiler/sparse/checkpatch warnings? - Did you check for new runtime errors/warnings from the driver? If the answer to any of those is "no", then I won't even bother reviewing anything. OK, that's enough ranting. Now for the more positive feedback. The code does work for me, for the mose part. There is some kind of initial problem though. When I first load the driver I get some kind of weird shifted grey screen. So I guess either we're misconfiguring something in the source, or we're not properly telling the sink what we're sending it. After another modeset the problem goes away and everything appears to work correctly. So congrats on making that happen \o/ > drm/i915: add config function for YCBCR420 outputs > drm/i915: prepare scaler for YCBCR420 modeset > drm/i915: prepare pipe for YCBCR420 output > drm/i915: prepare csc unit for YCBCR420 output > drm/i915: set colorspace for YCBCR420 outputs > drm/i915/glk: set HDMI 2.0 identifier These remaining i915 patches still have a few issues, the lack of state readout and the grey screen issue are the major ones. I've left more detailed feedback on the individual patches, but right now I don't have a clear idea what's causing the grey screen. Assuming the remaining issues I've highlighted get fixed, and the remaining patches won't get massively altered in the process, you can slap Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> onto the patches. > > 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_v6_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 | 438 +++++++++++++++++++++++++++++- > drivers/gpu/drm/drm_modes.c | 87 ++++++ > drivers/gpu/drm/drm_probe_helper.c | 4 + > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- > drivers/gpu/drm/i915/i915_reg.h | 3 + > drivers/gpu/drm/i915/intel_color.c | 46 +++- > drivers/gpu/drm/i915/intel_display.c | 26 ++ > drivers/gpu/drm/i915/intel_drv.h | 7 +- > drivers/gpu/drm/i915/intel_hdmi.c | 69 ++++- > drivers/gpu/drm/i915/intel_panel.c | 3 +- > drivers/gpu/drm/i915/intel_sdvo.c | 3 +- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 2 +- > drivers/gpu/drm/nouveau/nv50_display.c | 3 +- > 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/sun4i/sun4i_hdmi_enc.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_connector.h | 32 +++ > include/drm/drm_edid.h | 11 +- > include/drm/drm_modes.h | 11 + > 34 files changed, 746 insertions(+), 39 deletions(-) > > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer 2017-07-14 19:02 ` [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Ville Syrjälä @ 2017-07-15 5:03 ` Sharma, Shashank 0 siblings, 0 replies; 30+ messages in thread From: Sharma, Shashank @ 2017-07-15 5:03 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, dri-devel [-- Attachment #1.1: Type: text/plain, Size: 13777 bytes --] Regards Shashank On 7/15/2017 12:32 AM, Ville Syrjälä wrote: > On Thu, Jul 13, 2017 at 09:03:06PM +0530, Shashank Sharma wrote: >> Following YCBCR 4:4:4 and 4:2:2, YCBCR 4:2:0 is a new output format, >> which is currently supported on HDMI 2.0 sources/sinks. Due to lower >> chroma sub-sampling rate, YCBCR 4:2:0 can drive the video modes at half >> the pixel clock than YCBCR 4:4:4 or RGB 8:8:8 outputs. For example, a CEA >> 4K@60, RGB 8:8:8 mode needs a clock of appx 594Mhz, but it can be driven at >> 297Mhz using YCBCR 4:2:0 output. >> >> Of course, the lower rate of chroma subsampling, causes the quality of YCBCR >> 4:2:0 to be lower than YCBCR 4:4:4 or RGB 8:8:8. >> >> This patch series adds support for YCBCR 4:2:0 output in DRM layer. >> >> - First 2 patches, complete the CEA mode-db in drm driver, by adding >> new 4k modes. Current CEA mode-db contains 64 modes only (VIC 1-64), >> whereas CEA-861-F defined VICs up to 107, including 4k modes, from VIC >> range 93-107. First patch makes sure that inclusion of these modes doesn't >> break existing HDMI 1.4 monitors, across various drivers. >> >> - Next 5 patches focus on parsing new YCBCR 4:2:0 EDID blocks, and adding >> YCBCR 4:2:0 modes in connector. They also contain a prune function, which >> will cleanup the YCBCR 4:2:0 modes from list, if the connector doesn't >> declare them supported. >> >> - Next 2 patches add helper functions for identifing YCBCR 4:2:0 modes and >> setup the color space in AVI infoframes. >> >> - Next 6 patches add code for I915 layer handling of YCBCR 4:2:0 output. >> >> This patch series was initially published as a complete framework to handle >> all YCBCR outputs (4:4:4, 4:2:2, 4:2:0), but based on the code reviews, now >> its been published as YCBCR 4:2:0 handling series only. >> >> The previous discussion and reviews can be found here: >> V5: https://patchwork.freedesktop.org/series/26815/ >> V1-V4: https://patchwork.freedesktop.org/series/22683/ >> >> Now re-publishing this patch series as YCBCR 4:2:0 handling series here. >> V2: Addressed review comments from Ville >> This series dropped one patch in V2(patch 8 from V1), so 14 patches in V2 >> >> This series has been tested with drm-tip code with following setup: >> Source: Intel Geminilake device. >> Sink: ACER S277HK HDMI 2.0 monitor. >> Protocol testing: Astro VA-1844A HDMI analyzer. >> >> Shashank Sharma (14): >> drm: handle HDMI 2.0 VICs in AVI info-frames >> drm/edid: complete CEA modedb(VIC 1-107) >> drm/edid: parse sink information before CEA blocks >> drm/edid: cleanup patch for CEA extended-tag macro >> drm: add helper to validate YCBCR420 modes >> drm/edid: parse YCBCR420 videomodes from EDID >> drm/edid: parse ycbcr 420 deep color information >> drm: add helper functions for YCBCR420 handling > I just finished pushing the core bits into drm-misc-next. But I'm not > super happy how that went because there were still quite a few trivial > checkpatch warnings and indentation issues I had to fix up. All of that > should have been dealt with before even submitting the patches to the > list. Review bandwidth is a scarce resource so we don't want to squander > it on this sort of stuff. The whole patch series was checked with checkpatch every time it was sent for review. It had only 1 checkpatch warning, for which I had added an explanation in patch's header like: For patch 1: 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 Apart from this, here is the checkpatch output: ../scripts/checkpatch.pl *.patch --------------------------------------------------------- v8-0001-drm-handle-HDMI-2.0-VICs-in-AVI-info-frames.patch --------------------------------------------------------- WARNING: line over 80 characters #278: FILE: drivers/gpu/drm/i915/intel_sdvo.c:999: + &pipe_config->base.adjusted_mode, WARNING: line over 80 characters #332: FILE: drivers/gpu/drm/omapdrm/omap_encoder.c:88: + r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode, total: 0 errors, 2 warnings, 247 lines checked v8-0001-drm-handle-HDMI-2.0-VICs-in-AVI-info-frames.patch has style problems, please review. ---------------------------------------------------- v8-0002-drm-edid-complete-CEA-modedb-VIC-1-107.patch ---------------------------------------------------- total: 0 errors, 0 warnings, 245 lines checked v8-0002-drm-edid-complete-CEA-modedb-VIC-1-107.patch has no obvious style problems and is ready for submission. --------------------------------------------------------------- v8-0003-drm-edid-parse-sink-information-before-CEA-blocks.patch --------------------------------------------------------------- total: 0 errors, 0 warnings, 21 lines checked v8-0003-drm-edid-parse-sink-information-before-CEA-blocks.patch has no obvious style problems and is ready for submission. --------------------------------------------------------------- v8-0004-drm-edid-cleanup-patch-for-CEA-extended-tag-macro.patch --------------------------------------------------------------- total: 0 errors, 0 warnings, 33 lines checked v8-0004-drm-edid-cleanup-patch-for-CEA-extended-tag-macro.patch has no obvious style problems and is ready for submission. ------------------------------------------------------- v8-0005-drm-add-helper-to-validate-YCBCR420-modes.patch ------------------------------------------------------- total: 0 errors, 0 warnings, 110 lines checked v8-0005-drm-add-helper-to-validate-YCBCR420-modes.patch has no obvious style problems and is ready for submission. ---------------------------------------------------------- v8-0006-drm-edid-parse-YCBCR420-videomodes-from-EDID.patch ---------------------------------------------------------- total: 0 errors, 0 warnings, 228 lines checked v8-0006-drm-edid-parse-YCBCR420-videomodes-from-EDID.patch has no obvious style problems and is ready for submission. ------------------------------------------------------------- v8-0007-drm-edid-parse-ycbcr-420-deep-color-information.patch ------------------------------------------------------------- total: 0 errors, 0 warnings, 47 lines checked v8-0007-drm-edid-parse-ycbcr-420-deep-color-information.patch has no obvious style problems and is ready for submission. ------------------------------------------------------------ v8-0008-drm-add-helper-functions-for-YCBCR420-handling.patch ------------------------------------------------------------ total: 0 errors, 0 warnings, 73 lines checked v8-0008-drm-add-helper-functions-for-YCBCR420-handling.patch has no obvious style problems and is ready for submission. --------------------------------------------------------------- v8-0009-drm-i915-add-config-function-for-YCBCR420-outputs.patch --------------------------------------------------------------- total: 0 errors, 0 warnings, 89 lines checked v8-0009-drm-i915-add-config-function-for-YCBCR420-outputs.patch has no obvious style problems and is ready for submission. ---------------------------------------------------------- v8-0010-drm-i915-prepare-scaler-for-YCBCR420-modeset.patch ---------------------------------------------------------- total: 0 errors, 0 warnings, 58 lines checked v8-0010-drm-i915-prepare-scaler-for-YCBCR420-modeset.patch has no obvious style problems and is ready for submission. ------------------------------------------------------- v8-0011-drm-i915-prepare-pipe-for-YCBCR420-output.patch ------------------------------------------------------- total: 0 errors, 0 warnings, 28 lines checked v8-0011-drm-i915-prepare-pipe-for-YCBCR420-output.patch has no obvious style problems and is ready for submission. ----------------------------------------------------------- v8-0012-drm-i915-prepare-csc-unit-for-YCBCR420-output.patch ----------------------------------------------------------- total: 0 errors, 0 warnings, 85 lines checked v8-0012-drm-i915-prepare-csc-unit-for-YCBCR420-output.patch has no obvious style problems and is ready for submission. ---------------------------------------------------------- v8-0013-drm-i915-set-colorspace-for-YCBCR420-outputs.patch ---------------------------------------------------------- total: 0 errors, 0 warnings, 18 lines checked v8-0013-drm-i915-set-colorspace-for-YCBCR420-outputs.patch has no obvious style problems and is ready for submission. -------------------------------------------------- v8-0014-drm-i915-glk-set-HDMI-2.0-identifier.patch -------------------------------------------------- total: 0 errors, 0 warnings, 9 lines checked v8-0014-drm-i915-glk-set-HDMI-2.0-identifier.patch has no obvious style problems and is ready for submission. > Another issue I ran into when I tried to actually run the code. As soon > as I loaded the driver a warning and the accompanying backtrace from > the state checker flew past my terminal. That too should have been > caught before even sending the patches to the list. Yes, I had seen that, and I knew why is that happening, but I needed a discussion on what is the right way to fix it. It was about readouts of the state and pixel clock mismatch (as YCBCR420 clock = half). But at the same time, I am at fault, I should have mentioned this somewhere in the cover letter or I should have mentioned it to you on IRC. Sorry about that, My bad :( ! > So I think the next time someone asks me to review something I will > start by asking these basic questions: > - Did you configure your editor so that it automagically formats code correctly? > - Did you check for new compiler/sparse/checkpatch warnings? > - Did you check for new runtime errors/warnings from the driver? > > If the answer to any of those is "no", then I won't even bother > reviewing anything. > > OK, that's enough ranting. Now for the more positive feedback. > The code does work for me, for the mose part. There is some kind of > initial problem though. When I first load the driver I get some > kind of weird shifted grey screen. So I guess either we're > misconfiguring something in the source, or we're not properly telling > the sink what we're sending it. After another modeset the problem > goes away and everything appears to work correctly. So congrats > on making that happen \o/ Thanks, for the detailed review, and the time you invested on this. As such I had tested this whole series with this combination: - GLK system & KBL NUC system - 2 different HDMI 2.0 monitors ACER/Samsung - 1 HDMI 1.4 monitor (Dell) - 1 HDMI analyzer (Astro VA1844) - Mint desktop GUI - IGT (testdisplay) and I dint see any greying issue even once, but May be I will try to test with the way you are testing. > >> drm/i915: add config function for YCBCR420 outputs >> drm/i915: prepare scaler for YCBCR420 modeset >> drm/i915: prepare pipe for YCBCR420 output >> drm/i915: prepare csc unit for YCBCR420 output >> drm/i915: set colorspace for YCBCR420 outputs >> drm/i915/glk: set HDMI 2.0 identifier > These remaining i915 patches still have a few issues, the lack of > state readout and the grey screen issue are the major ones. I've > left more detailed feedback on the individual patches, but right > now I don't have a clear idea what's causing the grey screen. > > Assuming the remaining issues I've highlighted get fixed, and the > remaining patches won't get massively altered in the process, > you can slap > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > onto the patches. Thanks ! - Shashank > >> 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_v6_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 | 438 +++++++++++++++++++++++++++++- >> drivers/gpu/drm/drm_modes.c | 87 ++++++ >> drivers/gpu/drm/drm_probe_helper.c | 4 + >> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- >> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- >> drivers/gpu/drm/i915/i915_reg.h | 3 + >> drivers/gpu/drm/i915/intel_color.c | 46 +++- >> drivers/gpu/drm/i915/intel_display.c | 26 ++ >> drivers/gpu/drm/i915/intel_drv.h | 7 +- >> drivers/gpu/drm/i915/intel_hdmi.c | 69 ++++- >> drivers/gpu/drm/i915/intel_panel.c | 3 +- >> drivers/gpu/drm/i915/intel_sdvo.c | 3 +- >> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +- >> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 2 +- >> drivers/gpu/drm/nouveau/nv50_display.c | 3 +- >> 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/sun4i/sun4i_hdmi_enc.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_connector.h | 32 +++ >> include/drm/drm_edid.h | 11 +- >> include/drm/drm_modes.h | 11 + >> 34 files changed, 746 insertions(+), 39 deletions(-) >> >> -- >> 2.7.4 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel [-- Attachment #1.2: Type: text/html, Size: 16735 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2017-07-15 5:03 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-07-13 15:33 [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 01/14] drm: handle HDMI 2.0 VICs in AVI info-frames Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 02/14] drm/edid: complete CEA modedb(VIC 1-107) Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 03/14] drm/edid: parse sink information before CEA blocks Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 04/14] drm/edid: cleanup patch for CEA extended-tag macro Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 05/14] drm: add helper to validate YCBCR420 modes Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 06/14] drm/edid: parse YCBCR420 videomodes from EDID Shashank Sharma 2017-07-13 16:21 ` Ville Syrjälä 2017-07-14 4:04 ` Sharma, Shashank 2017-07-14 10:33 ` [PATCH v8 " Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 07/14] drm/edid: parse ycbcr 420 deep color information Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 08/14] drm: add helper functions for YCBCR420 handling Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 09/14] drm/i915: add config function for YCBCR420 outputs Shashank Sharma 2017-07-14 18:30 ` Ville Syrjälä 2017-07-15 4:40 ` Sharma, Shashank 2017-07-13 15:33 ` [PATCH v2 10/14] drm/i915: prepare scaler for YCBCR420 modeset Shashank Sharma 2017-07-14 18:30 ` Ville Syrjälä 2017-07-15 4:41 ` Sharma, Shashank 2017-07-13 15:33 ` [PATCH v2 11/14] drm/i915: prepare pipe for YCBCR420 output Shashank Sharma 2017-07-14 18:33 ` Ville Syrjälä 2017-07-15 4:45 ` Sharma, Shashank 2017-07-13 15:33 ` [PATCH v2 12/14] drm/i915: prepare csc unit " Shashank Sharma 2017-07-14 18:36 ` Ville Syrjälä 2017-07-15 4:46 ` Sharma, Shashank 2017-07-13 15:33 ` [PATCH v2 13/14] drm/i915: set colorspace for YCBCR420 outputs Shashank Sharma 2017-07-13 15:33 ` [PATCH v2 14/14] drm/i915/glk: set HDMI 2.0 identifier Shashank Sharma 2017-07-13 15:50 ` ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev2) Patchwork 2017-07-14 10:57 ` ✓ Fi.CI.BAT: success for YCBCR 4:2:0 handling in DRM layer (rev3) Patchwork 2017-07-14 19:02 ` [PATCH v2 00/14] YCBCR 4:2:0 handling in DRM layer Ville Syrjälä 2017-07-15 5:03 ` Sharma, Shashank
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox