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

[Nautiyal, Ankit K]

On 9/13/2022 1:45 PM, Ville Syrjälä wrote:
> On Tue, Sep 13, 2022 at 10:55:24AM +0530, Nautiyal, Ankit K wrote:
>> 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 at 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?
> If other output types end up getting some kind of converterts
> then yeah sink_format would work there as well. I think initally
> we should just populate it the same as output_format for all
> encoder types, and add it to the crtc state dump. Unfortunately
> I don't think we can add it to the state checker on account of
> the user being able to yank out the dongle mid-modeset.

Got it. Let me add sink_format in crtc_state and add it to crtc state dump.

So for encoder, will just set this to same as output_format.

For DP, will compute the DFP color configuration if required and 
accordingly set sink_format.

Will send the next version with the change.

Thanks & Regards,

Ankit

>
>> 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