Message ID | 20220225013055.9282-2-manasi.d.navare@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v6,1/2] drm/vrr: Set VRR capable prop only if it is attached to connector | expand |
On Thu, Feb 24, 2022 at 05:30:55PM -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) > v4: Move vrr_capable to after update modes call (Jani N) > Remove the redundant comment (Jan N) > v5: Fixes the regression on older platforms by reseting the VRR > only if HAS_VRR > v6: Remove the checks from driver, add in drm core before > setting VRR prop (Ville) > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Fixes: 390a1f8beb87 ("Revert "drm/i915/display/vrr: Reset VRR capable property on a long hpd") > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 1046e7fe310a..f96123b56935 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4455,6 +4455,10 @@ intel_dp_detect(struct drm_connector *connector, > memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance)); > memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd)); > > + /* Reset VRR Capable property on disconnect */ > + drm_connector_set_vrr_capable_property(connector, > + false); > + > if (intel_dp->is_mst) { > drm_dbg_kms(&dev_priv->drm, > "MST device may have disappeared %d vs %d\n", > @@ -4569,15 +4573,18 @@ 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; > > - if (intel_vrr_is_capable(connector)) > - drm_connector_set_vrr_capable_property(connector, > - true); > + num_modes = intel_connector_update_modes(connector, edid); > + vrr_capable = intel_vrr_is_capable(connector); > + drm_dbg_kms(&i915->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); As mentioned on irc I think intel_dp_(un)set_edid() would be a better place for these. > } > > /* Also add fixed mode, which may or may not be present in EDID */ > -- > 2.19.1
On Fri, Feb 25, 2022 at 11:13:35AM +0200, Ville Syrjälä wrote: > On Thu, Feb 24, 2022 at 05:30:55PM -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) > > v4: Move vrr_capable to after update modes call (Jani N) > > Remove the redundant comment (Jan N) > > v5: Fixes the regression on older platforms by reseting the VRR > > only if HAS_VRR > > v6: Remove the checks from driver, add in drm core before > > setting VRR prop (Ville) > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Fixes: 390a1f8beb87 ("Revert "drm/i915/display/vrr: Reset VRR capable property on a long hpd") > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index 1046e7fe310a..f96123b56935 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -4455,6 +4455,10 @@ intel_dp_detect(struct drm_connector *connector, > > memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance)); > > memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd)); > > > > + /* Reset VRR Capable property on disconnect */ > > + drm_connector_set_vrr_capable_property(connector, > > + false); > > + > > if (intel_dp->is_mst) { > > drm_dbg_kms(&dev_priv->drm, > > "MST device may have disappeared %d vs %d\n", > > @@ -4569,15 +4573,18 @@ 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; > > > > - if (intel_vrr_is_capable(connector)) > > - drm_connector_set_vrr_capable_property(connector, > > - true); > > + num_modes = intel_connector_update_modes(connector, edid); > > + vrr_capable = intel_vrr_is_capable(connector); > > + drm_dbg_kms(&i915->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); > > As mentioned on irc I think intel_dp_(un)set_edid() would be a better > place for these. Yes I guess I can add the above to check for vrr_capable and set it to true/false in intel_dp_set_edid() But the resetting part needs to happen outside of intel_dp_unset_edid so that we dont always set it to false before set_edid. It can just stay in intel_dp_detect where we clear other othing on connect status == disconnected. Is that fine? Manasi > > > } > > > > /* Also add fixed mode, which may or may not be present in EDID */ > > -- > > 2.19.1 > > -- > Ville Syrjälä > Intel
Hi Ville, Does it make sense to add the set prop in intel_dp_Set_edid but keep the reset to false in intel_dp_detect where we clear other parameters? Manasi On Fri, Feb 25, 2022 at 05:11:02PM -0800, Navare, Manasi wrote: > On Fri, Feb 25, 2022 at 11:13:35AM +0200, Ville Syrjälä wrote: > > On Thu, Feb 24, 2022 at 05:30:55PM -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) > > > v4: Move vrr_capable to after update modes call (Jani N) > > > Remove the redundant comment (Jan N) > > > v5: Fixes the regression on older platforms by reseting the VRR > > > only if HAS_VRR > > > v6: Remove the checks from driver, add in drm core before > > > setting VRR prop (Ville) > > > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Fixes: 390a1f8beb87 ("Revert "drm/i915/display/vrr: Reset VRR capable property on a long hpd") > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_dp.c | 15 +++++++++++---- > > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 1046e7fe310a..f96123b56935 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -4455,6 +4455,10 @@ intel_dp_detect(struct drm_connector *connector, > > > memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance)); > > > memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd)); > > > > > > + /* Reset VRR Capable property on disconnect */ > > > + drm_connector_set_vrr_capable_property(connector, > > > + false); > > > + > > > if (intel_dp->is_mst) { > > > drm_dbg_kms(&dev_priv->drm, > > > "MST device may have disappeared %d vs %d\n", > > > @@ -4569,15 +4573,18 @@ 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; > > > > > > - if (intel_vrr_is_capable(connector)) > > > - drm_connector_set_vrr_capable_property(connector, > > > - true); > > > + num_modes = intel_connector_update_modes(connector, edid); > > > + vrr_capable = intel_vrr_is_capable(connector); > > > + drm_dbg_kms(&i915->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); > > > > As mentioned on irc I think intel_dp_(un)set_edid() would be a better > > place for these. > > Yes I guess I can add the above to check for vrr_capable and set it to true/false in intel_dp_set_edid() > > But the resetting part needs to happen outside of intel_dp_unset_edid so that we dont always set it to false > before set_edid. It can just stay in intel_dp_detect where we clear other othing on connect status == disconnected. > Is that fine? > > Manasi > > > > > > } > > > > > > /* Also add fixed mode, which may or may not be present in EDID */ > > > -- > > > 2.19.1 > > > > -- > > Ville Syrjälä > > Intel
On Tue, Mar 01, 2022 at 11:30:52AM -0800, Navare, Manasi wrote: > Hi Ville, > > Does it make sense to add the set prop in intel_dp_Set_edid but keep the reset to false > in intel_dp_detect where we clear other parameters? We don't clear stuff in .detect(), or at least shouldn't. .detect() doesn't even get run when forcing the connector status. > > Manasi > > > On Fri, Feb 25, 2022 at 05:11:02PM -0800, Navare, Manasi wrote: > > On Fri, Feb 25, 2022 at 11:13:35AM +0200, Ville Syrjälä wrote: > > > On Thu, Feb 24, 2022 at 05:30:55PM -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) > > > > v4: Move vrr_capable to after update modes call (Jani N) > > > > Remove the redundant comment (Jan N) > > > > v5: Fixes the regression on older platforms by reseting the VRR > > > > only if HAS_VRR > > > > v6: Remove the checks from driver, add in drm core before > > > > setting VRR prop (Ville) > > > > > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Fixes: 390a1f8beb87 ("Revert "drm/i915/display/vrr: Reset VRR capable property on a long hpd") > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp.c | 15 +++++++++++---- > > > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > > index 1046e7fe310a..f96123b56935 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > @@ -4455,6 +4455,10 @@ intel_dp_detect(struct drm_connector *connector, > > > > memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance)); > > > > memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd)); > > > > > > > > + /* Reset VRR Capable property on disconnect */ > > > > + drm_connector_set_vrr_capable_property(connector, > > > > + false); > > > > + > > > > if (intel_dp->is_mst) { > > > > drm_dbg_kms(&dev_priv->drm, > > > > "MST device may have disappeared %d vs %d\n", > > > > @@ -4569,15 +4573,18 @@ 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; > > > > > > > > - if (intel_vrr_is_capable(connector)) > > > > - drm_connector_set_vrr_capable_property(connector, > > > > - true); > > > > + num_modes = intel_connector_update_modes(connector, edid); > > > > + vrr_capable = intel_vrr_is_capable(connector); > > > > + drm_dbg_kms(&i915->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); > > > > > > As mentioned on irc I think intel_dp_(un)set_edid() would be a better > > > place for these. > > > > Yes I guess I can add the above to check for vrr_capable and set it to true/false in intel_dp_set_edid() > > > > But the resetting part needs to happen outside of intel_dp_unset_edid so that we dont always set it to false > > before set_edid. It can just stay in intel_dp_detect where we clear other othing on connect status == disconnected. > > Is that fine? > > > > Manasi > > > > > > > > > } > > > > > > > > /* Also add fixed mode, which may or may not be present in EDID */ > > > > -- > > > > 2.19.1 > > > > > > -- > > > Ville Syrjälä > > > Intel
On Tue, Mar 01, 2022 at 09:34:54PM +0200, Ville Syrjälä wrote: > On Tue, Mar 01, 2022 at 11:30:52AM -0800, Navare, Manasi wrote: > > Hi Ville, > > > > Does it make sense to add the set prop in intel_dp_Set_edid but keep the reset to false > > in intel_dp_detect where we clear other parameters? > > We don't clear stuff in .detect(), or at least shouldn't. .detect() doesn't > even get run when forcing the connector status. Well we do clear some compliance variables there. But okay I can move the VRR rest stuff in intel_dp_unset_edid(), just that this will also get called evertime we set edid, it will always clear this first. Manasi > > > > > Manasi > > > > > > On Fri, Feb 25, 2022 at 05:11:02PM -0800, Navare, Manasi wrote: > > > On Fri, Feb 25, 2022 at 11:13:35AM +0200, Ville Syrjälä wrote: > > > > On Thu, Feb 24, 2022 at 05:30:55PM -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) > > > > > v4: Move vrr_capable to after update modes call (Jani N) > > > > > Remove the redundant comment (Jan N) > > > > > v5: Fixes the regression on older platforms by reseting the VRR > > > > > only if HAS_VRR > > > > > v6: Remove the checks from driver, add in drm core before > > > > > setting VRR prop (Ville) > > > > > > > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > Fixes: 390a1f8beb87 ("Revert "drm/i915/display/vrr: Reset VRR capable property on a long hpd") > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 15 +++++++++++---- > > > > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > index 1046e7fe310a..f96123b56935 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > @@ -4455,6 +4455,10 @@ intel_dp_detect(struct drm_connector *connector, > > > > > memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance)); > > > > > memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd)); > > > > > > > > > > + /* Reset VRR Capable property on disconnect */ > > > > > + drm_connector_set_vrr_capable_property(connector, > > > > > + false); > > > > > + > > > > > if (intel_dp->is_mst) { > > > > > drm_dbg_kms(&dev_priv->drm, > > > > > "MST device may have disappeared %d vs %d\n", > > > > > @@ -4569,15 +4573,18 @@ 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; > > > > > > > > > > - if (intel_vrr_is_capable(connector)) > > > > > - drm_connector_set_vrr_capable_property(connector, > > > > > - true); > > > > > + num_modes = intel_connector_update_modes(connector, edid); > > > > > + vrr_capable = intel_vrr_is_capable(connector); > > > > > + drm_dbg_kms(&i915->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); > > > > > > > > As mentioned on irc I think intel_dp_(un)set_edid() would be a better > > > > place for these. > > > > > > Yes I guess I can add the above to check for vrr_capable and set it to true/false in intel_dp_set_edid() > > > > > > But the resetting part needs to happen outside of intel_dp_unset_edid so that we dont always set it to false > > > before set_edid. It can just stay in intel_dp_detect where we clear other othing on connect status == disconnected. > > > Is that fine? > > > > > > Manasi > > > > > > > > > > > > } > > > > > > > > > > /* Also add fixed mode, which may or may not be present in EDID */ > > > > > -- > > > > > 2.19.1 > > > > > > > > -- > > > > Ville Syrjälä > > > > Intel > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 1046e7fe310a..f96123b56935 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -4455,6 +4455,10 @@ intel_dp_detect(struct drm_connector *connector, memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance)); memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd)); + /* Reset VRR Capable property on disconnect */ + drm_connector_set_vrr_capable_property(connector, + false); + if (intel_dp->is_mst) { drm_dbg_kms(&dev_priv->drm, "MST device may have disappeared %d vs %d\n", @@ -4569,15 +4573,18 @@ 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; - if (intel_vrr_is_capable(connector)) - drm_connector_set_vrr_capable_property(connector, - true); + num_modes = intel_connector_update_modes(connector, edid); + vrr_capable = intel_vrr_is_capable(connector); + drm_dbg_kms(&i915->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 */
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) v4: Move vrr_capable to after update modes call (Jani N) Remove the redundant comment (Jan N) v5: Fixes the regression on older platforms by reseting the VRR only if HAS_VRR v6: Remove the checks from driver, add in drm core before setting VRR prop (Ville) Cc: Jani Nikula <jani.nikula@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Fixes: 390a1f8beb87 ("Revert "drm/i915/display/vrr: Reset VRR capable property on a long hpd") Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)