On 11/15/2022 4:30 PM, Ville Syrjälä wrote:
On Tue, Nov 15, 2022 at 12:23:53PM +0530, Nautiyal, Ankit K wrote:
On 11/11/2022 2:36 AM, Ville Syrjälä wrote:
On Fri, Oct 28, 2022 at 03:14:05PM +0530, Ankit Nautiyal wrote:
The decision to use DFP output format conversion capabilities should be
during compute_config phase.
This patch uses the members of intel_dp->dfp to only store the
format conversion capabilities of the DP device and uses the crtc_state
sink_format member, to program the protocol-converter for
colorspace/format conversion.
v2: Use sink_format to determine the color conversion config for the
pcon (Ville).
v3: Fix typo: missing 'break' in switch case (lkp kernel test robot).
v4: Add helper to check if DP supports YCBCR420.
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_dp.c | 122 ++++++++++++++++--------
1 file changed, 84 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0e4f7b467970..95d0c653db7f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -790,6 +790,7 @@ intel_dp_output_format(struct intel_connector *connector,
enum intel_output_format sink_format)
{
struct intel_dp *intel_dp = intel_attached_dp(connector);
+ struct drm_i915_private *i915 = dp_to_i915(intel_dp);
if (!connector->base.ycbcr_420_allowed ||
sink_format != INTEL_OUTPUT_FORMAT_YCBCR420)
@@ -799,6 +800,10 @@ intel_dp_output_format(struct intel_connector *connector,
intel_dp->dfp.ycbcr_444_to_420)
return INTEL_OUTPUT_FORMAT_RGB;
+ /* Prefer 4:2:0 passthrough over 4:4:4->4:2:0 conversion */
+ if (DISPLAY_VER(i915) >= 11 && intel_dp->dfp.ycbcr420_passthrough)
+ return INTEL_OUTPUT_FORMAT_YCBCR420;
+
if (intel_dp->dfp.ycbcr_444_to_420)
return INTEL_OUTPUT_FORMAT_YCBCR444;
else
The else branch here is also 420, so the whole logic
here doesn't seem to flow entirely sensibly.
Thinking a bit more abstractly, could we restate
this whole problem as something more like this?
if (source_can_output(sink_format))
return sink_format;
if (source_can_output(RGB) &&
dfp_can_convert_from_rgb(sink_format))
return RGB;
if (source_can_output(YCBCR444) &&
dfp_can_convert_from_ycbcr444(sink_format))
return YCBCR444;
This make sense and will also help to add 422 support easier.
I am only seeing one problem: about how to have DSC considerations
during source_can_output( ).
Gen 11+ should support YCBCR420. So for any ycbcr420_only mode we should
have sink_format, and output_format : YCbCr420.
This works well for cases where DSC doesn't get in picture. When higher
modes like 8k60 ycbcr420_only are involved, we need to use DSC.
Currently, our DSC1.1 does not support YCbCr420. The problem is that we
go for, dsc_compute_config at a later time.
This issue might have been masked, due to the messy order of checks in
intel_dp_output_format.
Specifically With HDMI2.1 PCONs supporting color conv, for such a case
we can have output_format as RGB, and sink_format as YCbCr420.
The DSC compression with RGB and then configure PCON to Decompress,
conv. to YCbCr420.
So can we put a check in source_can_output for Gen11+ where DSC might be
required, so that with source_can_output(YCBCR420) returns false in such
case, where DSC is the only way?
I'm thinking it should work well enough without any extra checks
since we'll always try RGB before YCbC4 4:2:0. I guess the only
case where it could fail is if the sink only supports 4:2:0 for
that particular mode. Then we'd also try direct YCbC4 4:2:0 output
on icl+. Dunno if such sinks are still a thing when DSC is in the
picture.
There indeed are some HDMI 8k Panels that have 8k@60 in Ycbcr420 only mode.
These do not have DSC, so without DSC these can support 8k@60 only in
YCbCr420.
(We have a SONY XBR-Z8H in lab, and there are some in market from
Samsung too, which support 8k@60 in YCbcr420 only).
With PCON we can support these. As mentioned above, we need to send
compressed stream in RGB to PCON.
PCON decompresses, converts RGB444 to Ycbcr420. The sink is sent 8k@60
Ycbcr420 uncompressed.
Hmm. Do PCONs even support DSC + color conversion/etc. at the
same time? They'd have to decompress and potentially recompress
the data in that case.
Yes there are PCONs that support 3 modes of operations along with color
conversion.
DSC1.2 pass-through: A DSC1.2 compressed just gets forwarded to DSC1.2
supporting HDMI2.1sink.
DSC1.1->DSC1.2 : DSC1.1 compressed stream is decompressed and then
re-compressed again with DSC1.2 and forward to DSC1.2 supporting HDMI2.1
sink.
DSC1.x->uncompress: DSC1.x is decompressed and sent to HDMI sink that
does not support DSC.
(the case mentioned above, uses this 3rd option)
The problem with adding DSC checks into source_can_output() is
that we'd need to express those in a way that is also usable in
.mode_valid() since we'd want to use the same code there I think.
Looks like we do have some DSC stuff in there already, but it
doesn't seem to take that into account in the link bandwidth
check. So to me it looks like the whole DSC support is incomplete
anyway.
Hmm. We were not getting this earlier, due to the order in which
YCbCr420 sink_format was chosen.
When sink format isYCbCr420, and DFP supports RGB444->YCBCR420, always
go with the conv via PCON.
This seems crude though, because if source supports YCBCR20, it is
natural to go with that first, and if not then try conv via PCON.
DSC consideration and the above case of 8k@60 YCbcr420 makes this
problematic.
Regards,
Ankit
