Re: [Intel-gfx] [PATCH v2] drm/i915/display/vrr: Reset VRR capable property on a long hpd
On Mon, Jan 31, 2022 at 02:28:04PM +0200, Jani Nikula wrote:
> On Wed, 26 Jan 2022, Manasi Navare wrote:
> > With some VRR panels, user can turn VRR ON/OFF on the fly from the panel
> > settings.
> > When VRR is turned OFF ,sends a long HPD to the driver clearing the Ignore
> > MSA bit
> > in the DPCD. Currently the driver parses that onevery HPD but fails to reset
> > the corresponding VRR Capable Connector property.
> > Hence the userspace still sees this as VRR Capable panel which is incorrect.
> >
> > Fix this by explicitly resetting the connector property.
> >
> > v2: Reset vrr capable if status == connector_disconnected
> > v3: Use i915 and use bool vrr_capable (Jani Nikula)
> > Cc: Jani Nikula
> > Signed-off-by: Manasi Navare
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 16
> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 4d4579a301f6..687cb37bb22a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4446,6 +4446,12 @@ intel_dp_detect(struct drm_connector *connector,
> > memset(_dp->compliance, 0, sizeof(intel_dp->compliance));
> > memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> >
> > + /* Reset VRR Capable property */
> > + drm_dbg_kms(_priv->drm, "[CONNECTOR:%d:%s] VRR capable:
> > FALSE\n",
> > + connector->base.id, connector->name);
>
> Both the comment and the logging here seem excessive.
Okay will remove the comment, it is indeed redundant now
>
> > + drm_connector_set_vrr_capable_property(connector,
> > + false);
> > +
>
> Fits on one line.
Sure will change that
>
> > if (intel_dp->is_mst) {
> > drm_dbg_kms(_priv->drm,
> > "MST device may have disappeared %d vs
> > %d\n",
> > @@ -4560,15 +4566,17 @@ static int intel_dp_get_modes(struct drm_connector
> > *connector)
> > {
> > struct intel_connector *intel_connector = to_intel_connector(connector);
> > struct edid *edid;
> > + struct drm_i915_private *i915 = to_i915(connector->dev);
> > int num_modes = 0;
> >
> > edid = intel_connector->detect_edid;
> > if (edid) {
> > - num_modes = intel_connector_update_modes(connector, edid);
> > + bool vrr_capable = intel_vrr_is_capable(connector);
> >
> > - if (intel_vrr_is_capable(connector))
> > - drm_connector_set_vrr_capable_property(connector,
> > - true);
> > + num_modes = intel_connector_update_modes(connector, edid);
>
> intel_vrr_is_capable() probably needs to happen after
> intel_connector_update_modes(), right?
Thanks Jani yes that is correct, will move this after num_modes =
intel_connector_update_modes(connector, edid);
Manasi
>
> BR,
> Jani.
>
> > + drm_dbg_kms(>drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
> > + connector->base.id, connector->name,
> > yesno(vrr_capable));
> > + drm_connector_set_vrr_capable_property(connector, vrr_capable);
> > }
> >
> > /* Also add fixed mode, which may or may not be present in EDID */
>
> --
> Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v2] drm/i915/display/vrr: Reset VRR capable property on a long hpd
On Wed, 26 Jan 2022, Manasi Navare wrote:
> With some VRR panels, user can turn VRR ON/OFF on the fly from the panel
> settings.
> When VRR is turned OFF ,sends a long HPD to the driver clearing the Ignore
> MSA bit
> in the DPCD. Currently the driver parses that onevery HPD but fails to reset
> the corresponding VRR Capable Connector property.
> Hence the userspace still sees this as VRR Capable panel which is incorrect.
>
> Fix this by explicitly resetting the connector property.
>
> v2: Reset vrr capable if status == connector_disconnected
> v3: Use i915 and use bool vrr_capable (Jani Nikula)
> Cc: Jani Nikula
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4d4579a301f6..687cb37bb22a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4446,6 +4446,12 @@ intel_dp_detect(struct drm_connector *connector,
> memset(_dp->compliance, 0, sizeof(intel_dp->compliance));
> memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
>
> + /* Reset VRR Capable property */
> + drm_dbg_kms(_priv->drm, "[CONNECTOR:%d:%s] VRR capable:
> FALSE\n",
> + connector->base.id, connector->name);
Both the comment and the logging here seem excessive.
> + drm_connector_set_vrr_capable_property(connector,
> +false);
> +
Fits on one line.
> if (intel_dp->is_mst) {
> drm_dbg_kms(_priv->drm,
> "MST device may have disappeared %d vs
> %d\n",
> @@ -4560,15 +4566,17 @@ static int intel_dp_get_modes(struct drm_connector
> *connector)
> {
> struct intel_connector *intel_connector = to_intel_connector(connector);
> struct edid *edid;
> + struct drm_i915_private *i915 = to_i915(connector->dev);
> int num_modes = 0;
>
> edid = intel_connector->detect_edid;
> if (edid) {
> - num_modes = intel_connector_update_modes(connector, edid);
> + bool vrr_capable = intel_vrr_is_capable(connector);
>
> - if (intel_vrr_is_capable(connector))
> - drm_connector_set_vrr_capable_property(connector,
> -true);
> + num_modes = intel_connector_update_modes(connector, edid);
intel_vrr_is_capable() probably needs to happen after
intel_connector_update_modes(), right?
BR,
Jani.
> + drm_dbg_kms(>drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
> + connector->base.id, connector->name,
> yesno(vrr_capable));
> + drm_connector_set_vrr_capable_property(connector, vrr_capable);
> }
>
> /* Also add fixed mode, which may or may not be present in EDID */
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH v2] drm/i915/display/vrr: Reset VRR capable property on a long hpd
Hi Jani,
I have addressed your review comments, could you take a look at this patch,
this is needed by one of our customers.
Manasi
On Wed, Jan 26, 2022 at 11:53:04AM -0800, Manasi Navare wrote:
> With some VRR panels, user can turn VRR ON/OFF on the fly from the panel
> settings.
> When VRR is turned OFF ,sends a long HPD to the driver clearing the Ignore
> MSA bit
> in the DPCD. Currently the driver parses that onevery HPD but fails to reset
> the corresponding VRR Capable Connector property.
> Hence the userspace still sees this as VRR Capable panel which is incorrect.
>
> Fix this by explicitly resetting the connector property.
>
> v2: Reset vrr capable if status == connector_disconnected
> v3: Use i915 and use bool vrr_capable (Jani Nikula)
> Cc: Jani Nikula
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4d4579a301f6..687cb37bb22a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4446,6 +4446,12 @@ intel_dp_detect(struct drm_connector *connector,
> memset(_dp->compliance, 0, sizeof(intel_dp->compliance));
> memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
>
> + /* Reset VRR Capable property */
> + drm_dbg_kms(_priv->drm, "[CONNECTOR:%d:%s] VRR capable:
> FALSE\n",
> + connector->base.id, connector->name);
> + drm_connector_set_vrr_capable_property(connector,
> +false);
> +
> if (intel_dp->is_mst) {
> drm_dbg_kms(_priv->drm,
> "MST device may have disappeared %d vs
> %d\n",
> @@ -4560,15 +4566,17 @@ static int intel_dp_get_modes(struct drm_connector
> *connector)
> {
> struct intel_connector *intel_connector = to_intel_connector(connector);
> struct edid *edid;
> + struct drm_i915_private *i915 = to_i915(connector->dev);
> int num_modes = 0;
>
> edid = intel_connector->detect_edid;
> if (edid) {
> - num_modes = intel_connector_update_modes(connector, edid);
> + bool vrr_capable = intel_vrr_is_capable(connector);
>
> - if (intel_vrr_is_capable(connector))
> - drm_connector_set_vrr_capable_property(connector,
> -true);
> + num_modes = intel_connector_update_modes(connector, edid);
> + drm_dbg_kms(>drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
> + connector->base.id, connector->name,
> yesno(vrr_capable));
> + drm_connector_set_vrr_capable_property(connector, vrr_capable);
> }
>
> /* Also add fixed mode, which may or may not be present in EDID */
> --
> 2.19.1
>
[Intel-gfx] [PATCH v2] drm/i915/display/vrr: Reset VRR capable property on a long hpd
With some VRR panels, user can turn VRR ON/OFF on the fly from the panel
settings.
When VRR is turned OFF ,sends a long HPD to the driver clearing the Ignore MSA
bit
in the DPCD. Currently the driver parses that onevery HPD but fails to reset
the corresponding VRR Capable Connector property.
Hence the userspace still sees this as VRR Capable panel which is incorrect.
Fix this by explicitly resetting the connector property.
v2: Reset vrr capable if status == connector_disconnected
v3: Use i915 and use bool vrr_capable (Jani Nikula)
Cc: Jani Nikula
Signed-off-by: Manasi Navare
---
drivers/gpu/drm/i915/display/intel_dp.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 4d4579a301f6..687cb37bb22a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4446,6 +4446,12 @@ intel_dp_detect(struct drm_connector *connector,
memset(_dp->compliance, 0, sizeof(intel_dp->compliance));
memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
+ /* Reset VRR Capable property */
+ drm_dbg_kms(_priv->drm, "[CONNECTOR:%d:%s] VRR capable:
FALSE\n",
+ connector->base.id, connector->name);
+ drm_connector_set_vrr_capable_property(connector,
+ false);
+
if (intel_dp->is_mst) {
drm_dbg_kms(_priv->drm,
"MST device may have disappeared %d vs
%d\n",
@@ -4560,15 +4566,17 @@ static int intel_dp_get_modes(struct drm_connector
*connector)
{
struct intel_connector *intel_connector = to_intel_connector(connector);
struct edid *edid;
+ struct drm_i915_private *i915 = to_i915(connector->dev);
int num_modes = 0;
edid = intel_connector->detect_edid;
if (edid) {
- num_modes = intel_connector_update_modes(connector, edid);
+ bool vrr_capable = intel_vrr_is_capable(connector);
- if (intel_vrr_is_capable(connector))
- drm_connector_set_vrr_capable_property(connector,
- true);
+ num_modes = intel_connector_update_modes(connector, edid);
+ drm_dbg_kms(>drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
+ connector->base.id, connector->name,
yesno(vrr_capable));
+ drm_connector_set_vrr_capable_property(connector, vrr_capable);
}
/* Also add fixed mode, which may or may not be present in EDID */
--
2.19.1
