From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yakir Subject: Re: [PATCH] drm: bridge/dw_hdmi: Return num_modes in dw_hdmi_connector_get_modes Date: Fri, 05 Jun 2015 14:15:43 +0800 Message-ID: <55713E8F.5080202@rock-chips.com> References: <1433441076-20049-1-git-send-email-dianders@chromium.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1066530134==" Return-path: Received: from regular1.263xmail.com (regular1.263xmail.com [211.150.99.140]) by gabe.freedesktop.org (Postfix) with ESMTP id 7B3566E209 for ; Thu, 4 Jun 2015 23:15:48 -0700 (PDT) In-Reply-To: <1433441076-20049-1-git-send-email-dianders@chromium.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Doug Anderson , Philipp Zabel , Russell King , Thierry Reding Cc: andy.yan@rock-chips.com, fabio.estevam@freescale.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org This is a multi-part message in MIME format. --===============1066530134== Content-Type: multipart/alternative; boundary="------------020309050301000601000901" This is a multi-part message in MIME format. --------------020309050301000601000901 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Doug, =E5=9C=A8 2015/6/5 2:04, Doug Anderson =E5=86=99=E9=81=93: > The dw_hdmi_connector_get_modes() function accidentally forgets to > return the number of modes it added, although it has this information > stored in a local variable. Let's fix that. > > Without this fix, drm_helper_probe_single_connector_modes_merge_bits() > could get confused and always call drm_add_modes_noedid(). That's not > right. > > Signed-off-by: Doug Anderson Test-by: Yakir Yang Thanks for your patch, it looks good to me. I And I test it on my 1080p T= V, found that the 800x600@56Hz resolution which don't indicate in edid would= no longer report, that is right :) 33 31 connected HDMI-A 510x290 17 31 modes: name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) 800x600 60 800 840 968 1056 600 601 605 628 flags: phsync, pvsync;=20 type: driver 800x600 56 800 824 896 1024 600 601 603 625 flags: phsync, pvsync;=20 type: driver 640x480 60 640 656 752 800 480 490 492 525 flags: nhsync, nvsync;=20 type: driver 640x480 60 640 656 752 800 480 489 492 525 flags: nhsync, nvsync;=20 type: driver First detailed timing is preferred timing Established timings supported: 720x400@70Hz 640x480@60Hz 640x480@75Hz 800x600@60Hz 800x600@75Hz 1024x768@60Hz 1024x768@75Hz 1280x1024@75Hz Standard timings supported: 1152x864@75Hz 1280x1024@60Hz 1920x1080@60Hz Thanks ! - Yakir > --- > drivers/gpu/drm/bridge/dw_hdmi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/= dw_hdmi.c > index 594f84c..816d104 100644 > --- a/drivers/gpu/drm/bridge/dw_hdmi.c > +++ b/drivers/gpu/drm/bridge/dw_hdmi.c > @@ -1395,7 +1395,7 @@ static int dw_hdmi_connector_get_modes(struct drm= _connector *connector) > struct dw_hdmi *hdmi =3D container_of(connector, struct dw_hdmi, > connector); > struct edid *edid; > - int ret; > + int ret =3D 0; > =20 > if (!hdmi->ddc) > return 0; > @@ -1412,7 +1412,7 @@ static int dw_hdmi_connector_get_modes(struct drm= _connector *connector) > dev_dbg(hdmi->dev, "failed to get edid\n"); > } > =20 > - return 0; > + return ret; > } > =20 > static enum drm_mode_status --------------020309050301000601000901 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable Doug,

=E5=9C=A8 2015/6/5 2:04, Doug Anderson= =E5=86=99=E9=81=93:
The dw_hdmi_connector_get_modes() function accidenta=
lly forgets to
return the number of modes it added, although it has this information
stored in a local variable.  Let's fix that.

Without this fix, drm_helper_probe_single_connector_modes_merge_bits()
could get confused and always call drm_add_modes_noedid().  That's not
right.

Signed-off-by: Doug Anderson <dianders@chromium.org>

Test-by: Yakir Yang <ykk@rock-chips.com>

Thanks for your patch, it looks good to me. I And I test it on my 1080p TV,
found that the 800x600@56Hz resolution which don't indicate in edid would no
longer report, that is right :)

33=C2=A0=C2=A0=C2=A0 31=C2=A0=C2=A0=C2=A0 connected=C2=A0=C2=A0=C2=A0= HDMI-A=C2=A0=C2=A0=C2=A0 510x290=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 17= =C2=A0=C2=A0=C2=A0 31
=C2=A0 modes:
=C2=A0=C2=A0=C2=A0 name refresh (Hz) hdisp hss hse htot vdisp vss vse= vtot)
=C2=A0 800x600 60 800 840 968 1056 600 601 605 628 flags: phsync, pvs= ync; type: driver
=C2=A0 800x600 56 800 824 896 1024 600 601 60= 3 625 flags: phsync, pvsync; type: driver
=C2=A0 640x480 60 640 656 752 800 480 490 492 525 flags: nhsync, nvsy= nc; type: driver
=C2=A0 640x480 60 640 656 752 800 480 489 492= 525 flags: nhsync, nvsync; type: driver

First detailed timing is preferred timing
Established timings supported:
=C2=A0 720x400@70Hz
=C2=A0 640x480@60Hz
=C2=A0 640x480@75Hz
=C2=A0 800x600@60Hz
=C2=A0 800x600@75Hz
=C2=A0 1024x768@60Hz
=C2=A0 1024x768@75Hz
=C2=A0 1280x1024@75Hz
Standard timings supported:
=C2=A0 1152x864@75Hz
=C2=A0 1280x1024@60Hz
=C2=A0 1920x1080@60Hz

Thanks !
- Yakir

---
 drivers/gpu/drm/bridge/dw_hdmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw=
_hdmi.c
index 594f84c..816d104 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -1395,7 +1395,7 @@ static int dw_hdmi_connector_get_modes(struct drm_c=
onnector *connector)
 	struct dw_hdmi *hdmi =3D container_of(connector, struct dw_hdmi,
 					     connector);
 	struct edid *edid;
-	int ret;
+	int ret =3D 0;
=20
 	if (!hdmi->ddc)
 		return 0;
@@ -1412,7 +1412,7 @@ static int dw_hdmi_connector_get_modes(struct drm_c=
onnector *connector)
 		dev_dbg(hdmi->dev, "failed to get edid\n");
 	}
=20
-	return 0;
+	return ret;
 }
=20
 static enum drm_mode_status

--------------020309050301000601000901-- --===============1066530134== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1066530134==--