From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758594AbcH3M40 (ORCPT ); Tue, 30 Aug 2016 08:56:26 -0400 Received: from down.free-electrons.com ([37.187.137.238]:55539 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751629AbcH3M4Z (ORCPT ); Tue, 30 Aug 2016 08:56:25 -0400 Date: Tue, 30 Aug 2016 14:56:22 +0200 From: Maxime Ripard To: Chen-Yu Tsai Cc: David Airlie , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/sun4i: Clear encoder->bridge if a bridge is not found Message-ID: <20160830125622.GF18605@lukather> References: <20160830122223.21377-1-wens@csie.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="JSkcQAAxhB1h8DcT" Content-Disposition: inline In-Reply-To: <20160830122223.21377-1-wens@csie.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --JSkcQAAxhB1h8DcT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Tue, Aug 30, 2016 at 08:22:23PM +0800, Chen-Yu Tsai wrote: > The KMS helpers (drm_atomic_helper_check_modeset/mode_fixup) pass > encoder->bridge directly to drm_bridge_mode_fixup, which expects a > valid pointer, or NULL (in which case it just returns). >=20 > Clear encoder->bridge if a bridge is not found, instead of keeping > the ERR_PTR value. >=20 > Since other drm_bridge functions also follow this pattern of checking > for a non-NULL pointer, we can drop the ifs around the calls and just > pass the pointer directly. >=20 > Fixes: 894f5a9f4b4a ("drm/sun4i: Add bridge support") > Signed-off-by: Chen-Yu Tsai > --- > drivers/gpu/drm/sun4i/sun4i_rgb.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) >=20 > diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/su= n4i_rgb.c > index d4e52522ec53..ee0795152a33 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c > +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c > @@ -154,8 +154,7 @@ static void sun4i_rgb_encoder_enable(struct drm_encod= er *encoder) > if (!IS_ERR(tcon->panel)) > drm_panel_enable(tcon->panel); > =20 > - if (!IS_ERR(encoder->bridge)) > - drm_bridge_enable(encoder->bridge); > + drm_bridge_enable(encoder->bridge); > =20 > sun4i_tcon_channel_enable(tcon, 0); > } > @@ -170,8 +169,7 @@ static void sun4i_rgb_encoder_disable(struct drm_enco= der *encoder) > =20 > sun4i_tcon_channel_disable(tcon, 0); > =20 > - if (!IS_ERR(encoder->bridge)) > - drm_bridge_disable(encoder->bridge); > + drm_bridge_disable(encoder->bridge); I'd rather keep those changes, it makes it obvious that it's something optionnal, that can be set to NULL. > if (!IS_ERR(tcon->panel)) > drm_panel_disable(tcon->panel); > @@ -230,6 +228,9 @@ int sun4i_rgb_init(struct drm_device *drm) > return 0; > } > =20 > + if (IS_ERR(encoder->bridge)) > + encoder->bridge =3D NULL; > + And that could be the else condition of the if statement below. Thanks! Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --JSkcQAAxhB1h8DcT Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXxYJ2AAoJEBx+YmzsjxAgvpgQAJIjMv6cXTpeDnZEc0OMpzEM dKWgwVItJmazDuE3Bfqy5jDuVimpGlcwgwgzvvqq3kaktsvCq7c2BasKLzjibsHL jYCelfZuhar8rrgQhBg1aCibxO3QshyORHwbx4of4Fqrlb02v7q0c29Wl7KvhEEh ZrR9qHlqN3sWrsYNdf6wEhJp8uxieakKOwrbCmDi9m+30ipW3fccOAUOCVA+7hQJ SsErW/8+VPHRR5WEmdT0X9Imf6qS+0cgrkUTGlleU9WfyjuKMGcil9MRADyJqSRX meZVzJy1qfSqxtoSJt1sq5SaXaKFPArcfkghP8lZmoxuD4dLVA5lybdJhLBtadn/ JLE9/hpnMb/6M9iRmP0L1Qk2wTpNbAk07CQZ6yJc/Ars/weGEHPUF/vr+tOa11pi dPs8oHJU+CdAk8FLaD/piBt/YXyUujyb+ES7evvfEA2C35K+Ljr48JBvLaaDim80 eA7tb3qZ7I1QmkQdtAQNVoQmqBHBHGUyV4NKgzIdHUIo4ApIpeN72Awb11u7usgh xPXtlecJGdHN3vy5A00iOny8yXqojxbzE3jLjSKOaEmDLDhWONZohl+QcczadO9i H6ZBW3D8CIrvvGwNYn1F+I+7Tz/zcXJUC1T9nheCkdGPIxT6+BbIYo2GDArJCKAp QgbhkxpmBZqLaCejkSn7 =lnkj -----END PGP SIGNATURE----- --JSkcQAAxhB1h8DcT-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: maxime.ripard@free-electrons.com (Maxime Ripard) Date: Tue, 30 Aug 2016 14:56:22 +0200 Subject: [PATCH] drm/sun4i: Clear encoder->bridge if a bridge is not found In-Reply-To: <20160830122223.21377-1-wens@csie.org> References: <20160830122223.21377-1-wens@csie.org> Message-ID: <20160830125622.GF18605@lukather> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On Tue, Aug 30, 2016 at 08:22:23PM +0800, Chen-Yu Tsai wrote: > The KMS helpers (drm_atomic_helper_check_modeset/mode_fixup) pass > encoder->bridge directly to drm_bridge_mode_fixup, which expects a > valid pointer, or NULL (in which case it just returns). > > Clear encoder->bridge if a bridge is not found, instead of keeping > the ERR_PTR value. > > Since other drm_bridge functions also follow this pattern of checking > for a non-NULL pointer, we can drop the ifs around the calls and just > pass the pointer directly. > > Fixes: 894f5a9f4b4a ("drm/sun4i: Add bridge support") > Signed-off-by: Chen-Yu Tsai > --- > drivers/gpu/drm/sun4i/sun4i_rgb.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c > index d4e52522ec53..ee0795152a33 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c > +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c > @@ -154,8 +154,7 @@ static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder) > if (!IS_ERR(tcon->panel)) > drm_panel_enable(tcon->panel); > > - if (!IS_ERR(encoder->bridge)) > - drm_bridge_enable(encoder->bridge); > + drm_bridge_enable(encoder->bridge); > > sun4i_tcon_channel_enable(tcon, 0); > } > @@ -170,8 +169,7 @@ static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder) > > sun4i_tcon_channel_disable(tcon, 0); > > - if (!IS_ERR(encoder->bridge)) > - drm_bridge_disable(encoder->bridge); > + drm_bridge_disable(encoder->bridge); I'd rather keep those changes, it makes it obvious that it's something optionnal, that can be set to NULL. > if (!IS_ERR(tcon->panel)) > drm_panel_disable(tcon->panel); > @@ -230,6 +228,9 @@ int sun4i_rgb_init(struct drm_device *drm) > return 0; > } > > + if (IS_ERR(encoder->bridge)) > + encoder->bridge = NULL; > + And that could be the else condition of the if statement below. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxime Ripard Subject: Re: [PATCH] drm/sun4i: Clear encoder->bridge if a bridge is not found Date: Tue, 30 Aug 2016 14:56:22 +0200 Message-ID: <20160830125622.GF18605@lukather> References: <20160830122223.21377-1-wens@csie.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0463037163==" Return-path: Received: from mail.free-electrons.com (down.free-electrons.com [37.187.137.238]) by gabe.freedesktop.org (Postfix) with ESMTP id A2C4F6E039 for ; Tue, 30 Aug 2016 12:56:23 +0000 (UTC) In-Reply-To: <20160830122223.21377-1-wens@csie.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Chen-Yu Tsai Cc: linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org --===============0463037163== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="JSkcQAAxhB1h8DcT" Content-Disposition: inline --JSkcQAAxhB1h8DcT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Tue, Aug 30, 2016 at 08:22:23PM +0800, Chen-Yu Tsai wrote: > The KMS helpers (drm_atomic_helper_check_modeset/mode_fixup) pass > encoder->bridge directly to drm_bridge_mode_fixup, which expects a > valid pointer, or NULL (in which case it just returns). >=20 > Clear encoder->bridge if a bridge is not found, instead of keeping > the ERR_PTR value. >=20 > Since other drm_bridge functions also follow this pattern of checking > for a non-NULL pointer, we can drop the ifs around the calls and just > pass the pointer directly. >=20 > Fixes: 894f5a9f4b4a ("drm/sun4i: Add bridge support") > Signed-off-by: Chen-Yu Tsai > --- > drivers/gpu/drm/sun4i/sun4i_rgb.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) >=20 > diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/su= n4i_rgb.c > index d4e52522ec53..ee0795152a33 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c > +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c > @@ -154,8 +154,7 @@ static void sun4i_rgb_encoder_enable(struct drm_encod= er *encoder) > if (!IS_ERR(tcon->panel)) > drm_panel_enable(tcon->panel); > =20 > - if (!IS_ERR(encoder->bridge)) > - drm_bridge_enable(encoder->bridge); > + drm_bridge_enable(encoder->bridge); > =20 > sun4i_tcon_channel_enable(tcon, 0); > } > @@ -170,8 +169,7 @@ static void sun4i_rgb_encoder_disable(struct drm_enco= der *encoder) > =20 > sun4i_tcon_channel_disable(tcon, 0); > =20 > - if (!IS_ERR(encoder->bridge)) > - drm_bridge_disable(encoder->bridge); > + drm_bridge_disable(encoder->bridge); I'd rather keep those changes, it makes it obvious that it's something optionnal, that can be set to NULL. > if (!IS_ERR(tcon->panel)) > drm_panel_disable(tcon->panel); > @@ -230,6 +228,9 @@ int sun4i_rgb_init(struct drm_device *drm) > return 0; > } > =20 > + if (IS_ERR(encoder->bridge)) > + encoder->bridge =3D NULL; > + And that could be the else condition of the if statement below. Thanks! Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --JSkcQAAxhB1h8DcT Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXxYJ2AAoJEBx+YmzsjxAgvpgQAJIjMv6cXTpeDnZEc0OMpzEM dKWgwVItJmazDuE3Bfqy5jDuVimpGlcwgwgzvvqq3kaktsvCq7c2BasKLzjibsHL jYCelfZuhar8rrgQhBg1aCibxO3QshyORHwbx4of4Fqrlb02v7q0c29Wl7KvhEEh ZrR9qHlqN3sWrsYNdf6wEhJp8uxieakKOwrbCmDi9m+30ipW3fccOAUOCVA+7hQJ SsErW/8+VPHRR5WEmdT0X9Imf6qS+0cgrkUTGlleU9WfyjuKMGcil9MRADyJqSRX meZVzJy1qfSqxtoSJt1sq5SaXaKFPArcfkghP8lZmoxuD4dLVA5lybdJhLBtadn/ JLE9/hpnMb/6M9iRmP0L1Qk2wTpNbAk07CQZ6yJc/Ars/weGEHPUF/vr+tOa11pi dPs8oHJU+CdAk8FLaD/piBt/YXyUujyb+ES7evvfEA2C35K+Ljr48JBvLaaDim80 eA7tb3qZ7I1QmkQdtAQNVoQmqBHBHGUyV4NKgzIdHUIo4ApIpeN72Awb11u7usgh xPXtlecJGdHN3vy5A00iOny8yXqojxbzE3jLjSKOaEmDLDhWONZohl+QcczadO9i H6ZBW3D8CIrvvGwNYn1F+I+7Tz/zcXJUC1T9nheCkdGPIxT6+BbIYo2GDArJCKAp QgbhkxpmBZqLaCejkSn7 =lnkj -----END PGP SIGNATURE----- --JSkcQAAxhB1h8DcT-- --===============0463037163== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0463037163==--