[Intel-gfx] [RFC V3 0/2] Attach and Set vrr_enabled property

[Modem, Bhanuprakash]

On Tue-31-05-2022 07:26 pm, Daniel Vetter wrote:
> On Tue, May 17, 2022 at 12:56:34PM +0530, Bhanuprakash Modem wrote:
>> This series will add a support to set the vrr_enabled property for
>> crtc based on the platform support and the request from userspace.
>> And userspace can also query to get the status of "vrr_enabled".
>>
>> Test-with: 20220422075223.2792586-2-bhanuprakash.modem at intel.com
>>
>> Bhanuprakash Modem (2):
>>    drm/vrr: Attach vrr_enabled property to the drm crtc
>>    drm/i915/vrr: Set drm crtc vrr_enabled property
> 
> I'm rather confused by this patch set >
> - This seems to move the property from connector to crtc without any
>    justification. For uapi that we want to have standardized (anything
>    around kms really) that's no good, unless there's really a mandatory
>    semantic reason pls stick to existing uapi.

Thanks for the reply.

No, we are not moving any property from connector to crtc.
"vrr_capable" belongs to connector & "vrr_enabled" belongs to crtc.

I am trying to manipulate this "vrr_enabled" property.

> 
> - If the driver interface doesn't fit (maybe the helper should be on the
>    crtc and adjust the property for all connector) pls roll that change out
>    to all drivers.
> 
> - This is uapi, so needs igt tests and userspace. For igts we should make
>    sure they're generic so that they apply across all drivers which already
>    support this property, and not just create new intel-only testcases.

Yeah, IGT patches are already in ML:
https://patchwork.freedesktop.org/series/100539/

> 
> - Finally the property is set up, but not wired through. Or at least I'm
>    not seeing how this can even work.
> 
> So no idea what exactly you're aiming for here and what kind of comments
> you want, but this doesn't look like it's on the right path at all.

This "vrr_enabled" prop was userspace write only & driver read only prop 
which would be used by the userspace to request to enable the VRR on 
that CRTC.

Now I would like to modify this prop to be used as a bidirectional 
property. So, that userspace can request to enable the VRR and also to 
get the status of VRR on that CRTC.

Manasi is already replied to the Patch [1/2] in this series.

- Bhanu

> 
> Cheers, Daniel
> 
> 
>>
>>   drivers/gpu/drm/drm_crtc.c               | 26 ++++++++++++++++++++++++
>>   drivers/gpu/drm/drm_mode_config.c        |  2 +-
>>   drivers/gpu/drm/i915/display/intel_vrr.c |  8 ++++++++
>>   include/drm/drm_crtc.h                   |  3 +++
>>   4 files changed, 38 insertions(+), 1 deletion(-)
>>
>> --
>> 2.35.1
>>
>