Re: [RFC V3 0/2] Attach and Set vrr_enabled property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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






[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux