Re: [Intel-gfx] [PATCH v2 4/9] drm/i915/display: Add new members to configure PCON color conversion

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 9/13/2022 12:25 AM, Ville Syrjälä wrote:
> On Thu, Sep 01, 2022 at 11:30:56AM +0530, Ankit Nautiyal wrote:
>> The decision to use DFP output format conversion capabilities should be
>> during compute_config phase.
>>
>> This patch adds new members to crtc_state to help configure the DFP
>> output related conversions. Also adds a member to intel_dp to store
>> ycbcr420 pass through capability.
>>
>> This will help to store only the format conversion capabilities of the
>> DP device in intel_dp->dfp, and use crtc_state to compute and store the
>> configuration for color/format conversion for a given mode.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display_types.h | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 0da9b208d56e..065ed19a5dd3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1311,6 +1311,12 @@ struct intel_crtc_state {
>>   
>>   	/* for loading single buffered registers during vblank */
>>   	struct drm_vblank_work vblank_work;
>> +
>> +	/* DP DFP color configuration */
>> +	struct {
>> +		bool rgb_to_ycbcr;
>> +		bool ycbcr_444_to_420;
>> +	} dp_dfp_config;
> Hmm. I'm thinking just something like
>   enum intel_output_format sink_format;
> might be easier to deal with.

Thanks Ville for looking into this.

We indeed can compute the final sink_format during compute config (based 
on pcon color conversion capability + platform capability)

IIUC, then during config_protocol_converter we need to compute what 
conversion need to be done based on crtc->output_format and 
crtc->sink_format

eg. For sink_format YCBCR420:

1. if output_format is RGB -> configure  : rgb_to_ycbcr420 and 
ycbcr444_to_420

2. if output_format is YCBCR444 -> configure  : ycbcr444_to_420

3. if output_format is YCBCR420 -> configure : Do nothing YCBCR420 
passthrough

So we store the sink_format and configuration is implicit in this case.

The idea with the dp_dfp_config was to store the conversion config to be 
done by PCON with given output_format.

The sink_format in that case is implicit.

I am open to storing the sink_format too.

Can this be handy for other encoder as well, or this will be used for DP 
DFPs only?


Regards,

Ankit



>>   };
>>   
>>   enum intel_pipe_crc_source {
>> @@ -1704,6 +1710,7 @@ struct intel_dp {
>>   		int pcon_max_frl_bw;
>>   		u8 max_bpc;
>>   		bool ycbcr_444_to_420;
>> +		bool ycbcr420_passthrough;
>>   		bool rgb_to_ycbcr;
>>   	} dfp;
>>   
>> -- 
>> 2.25.1