drm_dp_dpcd_read/write already has debug error message. Drop redundant error messages which gives false status even if correct value is read in drm_dp_dpcd_read().
Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and recovery") Cc: Swati Sharma swati2.sharma@intel.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com Cc: Uma Shankar uma.shankar@intel.com (v2) Cc: Jani Nikula jani.nikula@intel.com Cc: "Ville Syrj_l_" ville.syrjala@linux.intel.com Cc: Imre Deak imre.deak@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Cc: Uma Shankar uma.shankar@intel.com Cc: "Jos_ Roberto de Souza" jose.souza@intel.com Cc: Sean Paul seanpaul@chromium.org Cc: stable@vger.kernel.org # v5.12+
Link: https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.... Signed-off-by: Swati Sharma swati2.sharma@intel.com --- drivers/gpu/drm/i915/display/intel_dp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index c386ef8eb200..5c84f51ad41d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct intel_dp *intel_dp) return;
if (drm_dp_dpcd_readb(&intel_dp->aux, - DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) { - drm_dbg_kms(&i915->drm, "Error in reading link service irq vector\n"); + DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) return; - }
if (drm_dp_dpcd_writeb(&intel_dp->aux, - DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) { - drm_dbg_kms(&i915->drm, "Error in writing link service irq vector\n"); + DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) return; - }
if (val & HDMI_LINK_STATUS_CHANGED) intel_dp_handle_hdmi_link_status_change(intel_dp);
On Thu, 12 Aug 2021, Swati Sharma swati2.sharma@intel.com wrote:
drm_dp_dpcd_read/write already has debug error message. Drop redundant error messages which gives false status even if correct value is read in drm_dp_dpcd_read().
I guess the only problem is it gets harder to associate the preceding low level error messages with intel_dp_check_link_service_irq(). *shrug*
Reviewed-by: Jani Nikula jani.nikula@intel.com
Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and recovery") Cc: Swati Sharma swati2.sharma@intel.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com Cc: Uma Shankar uma.shankar@intel.com (v2) Cc: Jani Nikula jani.nikula@intel.com Cc: "Ville Syrj_l_" ville.syrjala@linux.intel.com Cc: Imre Deak imre.deak@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Cc: Uma Shankar uma.shankar@intel.com Cc: "Jos_ Roberto de Souza" jose.souza@intel.com Cc: Sean Paul seanpaul@chromium.org Cc: stable@vger.kernel.org # v5.12+
Link: https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.... Signed-off-by: Swati Sharma swati2.sharma@intel.com
drivers/gpu/drm/i915/display/intel_dp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index c386ef8eb200..5c84f51ad41d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct intel_dp *intel_dp) return; if (drm_dp_dpcd_readb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) {drm_dbg_kms(&i915->drm, "Error in reading link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val)
- }
if (drm_dp_dpcd_writeb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {drm_dbg_kms(&i915->drm, "Error in writing link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1)
- }
if (val & HDMI_LINK_STATUS_CHANGED) intel_dp_handle_hdmi_link_status_change(intel_dp);
On 13-Aug-21 1:16 PM, Jani Nikula wrote:
On Thu, 12 Aug 2021, Swati Sharma swati2.sharma@intel.com wrote:
drm_dp_dpcd_read/write already has debug error message. Drop redundant error messages which gives false status even if correct value is read in drm_dp_dpcd_read().
I guess the only problem is it gets harder to associate the preceding low level error messages with intel_dp_check_link_service_irq(). *shrug*
Reviewed-by: Jani Nikula jani.nikula@intel.com
Thanks Jani for the review. Can you please merge?
Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and recovery") Cc: Swati Sharma swati2.sharma@intel.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com Cc: Uma Shankar uma.shankar@intel.com (v2) Cc: Jani Nikula jani.nikula@intel.com Cc: "Ville Syrj_l_" ville.syrjala@linux.intel.com Cc: Imre Deak imre.deak@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Cc: Uma Shankar uma.shankar@intel.com Cc: "Jos_ Roberto de Souza" jose.souza@intel.com Cc: Sean Paul seanpaul@chromium.org Cc: stable@vger.kernel.org # v5.12+
Link: https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.... Signed-off-by: Swati Sharma swati2.sharma@intel.com
drivers/gpu/drm/i915/display/intel_dp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index c386ef8eb200..5c84f51ad41d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct intel_dp *intel_dp) return; if (drm_dp_dpcd_readb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) {drm_dbg_kms(&i915->drm, "Error in reading link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val)
- }
if (drm_dp_dpcd_writeb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {drm_dbg_kms(&i915->drm, "Error in writing link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1)
- }
if (val & HDMI_LINK_STATUS_CHANGED) intel_dp_handle_hdmi_link_status_change(intel_dp);
On Mon, 16 Aug 2021, "Sharma, Swati2" swati2.sharma@intel.com wrote:
On 13-Aug-21 1:16 PM, Jani Nikula wrote:
On Thu, 12 Aug 2021, Swati Sharma swati2.sharma@intel.com wrote:
drm_dp_dpcd_read/write already has debug error message. Drop redundant error messages which gives false status even if correct value is read in drm_dp_dpcd_read().
I guess the only problem is it gets harder to associate the preceding low level error messages with intel_dp_check_link_service_irq(). *shrug*
Reviewed-by: Jani Nikula jani.nikula@intel.com
Thanks Jani for the review. Can you please merge?
There was another version with open review?
BR, Jani.
Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and recovery") Cc: Swati Sharma swati2.sharma@intel.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com Cc: Uma Shankar uma.shankar@intel.com (v2) Cc: Jani Nikula jani.nikula@intel.com Cc: "Ville Syrj_l_" ville.syrjala@linux.intel.com Cc: Imre Deak imre.deak@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Cc: Uma Shankar uma.shankar@intel.com Cc: "Jos_ Roberto de Souza" jose.souza@intel.com Cc: Sean Paul seanpaul@chromium.org Cc: stable@vger.kernel.org # v5.12+
Link: https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.... Signed-off-by: Swati Sharma swati2.sharma@intel.com
drivers/gpu/drm/i915/display/intel_dp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index c386ef8eb200..5c84f51ad41d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct intel_dp *intel_dp) return; if (drm_dp_dpcd_readb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) {drm_dbg_kms(&i915->drm, "Error in reading link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val)
- }
if (drm_dp_dpcd_writeb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {drm_dbg_kms(&i915->drm, "Error in writing link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1)
- }
if (val & HDMI_LINK_STATUS_CHANGED) intel_dp_handle_hdmi_link_status_change(intel_dp);
On 16-Aug-21 5:40 PM, Jani Nikula wrote:
On Mon, 16 Aug 2021, "Sharma, Swati2" swati2.sharma@intel.com wrote:
On 13-Aug-21 1:16 PM, Jani Nikula wrote:
On Thu, 12 Aug 2021, Swati Sharma swati2.sharma@intel.com wrote:
drm_dp_dpcd_read/write already has debug error message. Drop redundant error messages which gives false status even if correct value is read in drm_dp_dpcd_read().
I guess the only problem is it gets harder to associate the preceding low level error messages with intel_dp_check_link_service_irq(). *shrug*
Reviewed-by: Jani Nikula jani.nikula@intel.com
Thanks Jani for the review. Can you please merge?
There was another version with open review?
Yes. https://patchwork.freedesktop.org/series/93025/#rev3 Should I add debug prints how Imre suggested in other IRQ func to make it generic or should it be dropped from here too? Quoting imre "Yes, that's why I suggested to return for the '0 value read' case without any message printed, but still keep the message for the case when the drm_dp_dpcd_readb() fails." "Ok, it's good to keep them in sync at least, so I'm ok with removing the debug messages from here too."
Please let me know what is the better approach.
BR, Jani.
Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and recovery") Cc: Swati Sharma swati2.sharma@intel.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com Cc: Uma Shankar uma.shankar@intel.com (v2) Cc: Jani Nikula jani.nikula@intel.com Cc: "Ville Syrj_l_" ville.syrjala@linux.intel.com Cc: Imre Deak imre.deak@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Cc: Uma Shankar uma.shankar@intel.com Cc: "Jos_ Roberto de Souza" jose.souza@intel.com Cc: Sean Paul seanpaul@chromium.org Cc: stable@vger.kernel.org # v5.12+
Link: https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.... Signed-off-by: Swati Sharma swati2.sharma@intel.com
drivers/gpu/drm/i915/display/intel_dp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index c386ef8eb200..5c84f51ad41d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct intel_dp *intel_dp) return; if (drm_dp_dpcd_readb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) {drm_dbg_kms(&i915->drm, "Error in reading link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) return;
- } if (drm_dp_dpcd_writeb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {drm_dbg_kms(&i915->drm, "Error in writing link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) return;
- } if (val & HDMI_LINK_STATUS_CHANGED) intel_dp_handle_hdmi_link_status_change(intel_dp);
On Mon, 16 Aug 2021, "Sharma, Swati2" swati2.sharma@intel.com wrote:
On 16-Aug-21 5:40 PM, Jani Nikula wrote:
On Mon, 16 Aug 2021, "Sharma, Swati2" swati2.sharma@intel.com wrote:
On 13-Aug-21 1:16 PM, Jani Nikula wrote:
On Thu, 12 Aug 2021, Swati Sharma swati2.sharma@intel.com wrote:
drm_dp_dpcd_read/write already has debug error message. Drop redundant error messages which gives false status even if correct value is read in drm_dp_dpcd_read().
I guess the only problem is it gets harder to associate the preceding low level error messages with intel_dp_check_link_service_irq(). *shrug*
Reviewed-by: Jani Nikula jani.nikula@intel.com
Thanks Jani for the review. Can you please merge?
There was another version with open review?
Yes. https://patchwork.freedesktop.org/series/93025/#rev3 Should I add debug prints how Imre suggested in other IRQ func to make it generic or should it be dropped from here too? Quoting imre "Yes, that's why I suggested to return for the '0 value read' case without any message printed, but still keep the message for the case when the drm_dp_dpcd_readb() fails." "Ok, it's good to keep them in sync at least, so I'm ok with removing the debug messages from here too."
Please let me know what is the better approach.
IMO just nuke them.
BR, Jani
BR, Jani.
Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and recovery") Cc: Swati Sharma swati2.sharma@intel.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com Cc: Uma Shankar uma.shankar@intel.com (v2) Cc: Jani Nikula jani.nikula@intel.com Cc: "Ville Syrj_l_" ville.syrjala@linux.intel.com Cc: Imre Deak imre.deak@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Cc: Uma Shankar uma.shankar@intel.com Cc: "Jos_ Roberto de Souza" jose.souza@intel.com Cc: Sean Paul seanpaul@chromium.org Cc: stable@vger.kernel.org # v5.12+
Link: https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.... Signed-off-by: Swati Sharma swati2.sharma@intel.com
drivers/gpu/drm/i915/display/intel_dp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index c386ef8eb200..5c84f51ad41d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct intel_dp *intel_dp) return; if (drm_dp_dpcd_readb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) {drm_dbg_kms(&i915->drm, "Error in reading link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) return;
- } if (drm_dp_dpcd_writeb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {drm_dbg_kms(&i915->drm, "Error in writing link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) return;
- } if (val & HDMI_LINK_STATUS_CHANGED) intel_dp_handle_hdmi_link_status_change(intel_dp);
On 16-Aug-21 5:58 PM, Jani Nikula wrote:
On Mon, 16 Aug 2021, "Sharma, Swati2" swati2.sharma@intel.com wrote:
On 16-Aug-21 5:40 PM, Jani Nikula wrote:
On Mon, 16 Aug 2021, "Sharma, Swati2" swati2.sharma@intel.com wrote:
On 13-Aug-21 1:16 PM, Jani Nikula wrote:
On Thu, 12 Aug 2021, Swati Sharma swati2.sharma@intel.com wrote:
drm_dp_dpcd_read/write already has debug error message. Drop redundant error messages which gives false status even if correct value is read in drm_dp_dpcd_read().
I guess the only problem is it gets harder to associate the preceding low level error messages with intel_dp_check_link_service_irq(). *shrug*
Reviewed-by: Jani Nikula jani.nikula@intel.com
Thanks Jani for the review. Can you please merge?
There was another version with open review?
Yes. https://patchwork.freedesktop.org/series/93025/#rev3 Should I add debug prints how Imre suggested in other IRQ func to make it generic or should it be dropped from here too? Quoting imre "Yes, that's why I suggested to return for the '0 value read' case without any message printed, but still keep the message for the case when the drm_dp_dpcd_readb() fails." "Ok, it's good to keep them in sync at least, so I'm ok with removing the debug messages from here too."
Please let me know what is the better approach.
IMO just nuke them.
Thanks Jani N. Imre agrees on same. Can we merge now? https://patchwork.freedesktop.org/series/93025/#rev3
BR, Jani
BR, Jani.
Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and recovery") Cc: Swati Sharma swati2.sharma@intel.com Cc: Ankit Nautiyal ankit.k.nautiyal@intel.com Cc: Uma Shankar uma.shankar@intel.com (v2) Cc: Jani Nikula jani.nikula@intel.com Cc: "Ville Syrj_l_" ville.syrjala@linux.intel.com Cc: Imre Deak imre.deak@intel.com Cc: Manasi Navare manasi.d.navare@intel.com Cc: Uma Shankar uma.shankar@intel.com Cc: "Jos_ Roberto de Souza" jose.souza@intel.com Cc: Sean Paul seanpaul@chromium.org Cc: stable@vger.kernel.org # v5.12+
Link: https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.... Signed-off-by: Swati Sharma swati2.sharma@intel.com
drivers/gpu/drm/i915/display/intel_dp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index c386ef8eb200..5c84f51ad41d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct intel_dp *intel_dp) return; if (drm_dp_dpcd_readb(&intel_dp->aux,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) {drm_dbg_kms(&i915->drm, "Error in reading link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, &val) != 1 || !val) return;
- }
if (drm_dp_dpcd_writeb(&intel_dp->aux,DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {drm_dbg_kms(&i915->drm, "Error in writing link service irq vector\n");
DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) return;
- }
if (val & HDMI_LINK_STATUS_CHANGED) intel_dp_handle_hdmi_link_status_change(intel_dp);
linux-stable-mirror@lists.linaro.org
-
Jani Nikula -
Sharma, Swati2 -
Swati Sharma