From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Paul Subject: Re: [PATCH] drm/bridge: adv7511: Fix low refresh rate selection Date: Tue, 14 May 2019 14:12:57 -0400 Message-ID: <20190514181257.GS17077@art_vandelay> References: <20190424132210.26338-1-matt.redfearn@thinci.com> <20190511193226.GO13043@pendragon.ideasonboard.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20190511193226.GO13043@pendragon.ideasonboard.com> Sender: linux-kernel-owner@vger.kernel.org To: Laurent Pinchart Cc: Matt Redfearn , Archit Taneja , David Airlie , Kieran Bingham , "linux-kernel@vger.kernel.org" , Matthew Redfearn , Jia-Ju Bai , Sean Paul , "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org On Sat, May 11, 2019 at 10:32:26PM +0300, Laurent Pinchart wrote: > Hi Matt, > > Thank you for the patch. > > On Wed, Apr 24, 2019 at 01:22:27PM +0000, Matt Redfearn wrote: > > The driver currently sets register 0xfb (Low Refresh Rate) based on the > > value of mode->vrefresh. Firstly, this field is specified to be in Hz, > > but the magic numbers used by the code are Hz * 1000. This essentially > > leads to the low refresh rate always being set to 0x01, since the > > vrefresh value will always be less than 24000. Fix the magic numbers to > > be in Hz. > > Secondly, according to the comment in drm_modes.h, the field is not > > supposed to be used in a functional way anyway. Instead, use the helper > > function drm_mode_vrefresh(). > > > > Fixes: 9c8af882bf12 ("drm: Add adv7511 encoder driver") > > Signed-off-by: Matt Redfearn > > Wow, a 4.5 year old bug fix, nice :-) > > Reviewed-by: Laurent Pinchart > Applied to drm-misc-next-fixes. Thanks! Sean > > --- > > > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > index 85c2d407a52..e7ddd3e3db9 100644 > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > @@ -747,11 +747,11 @@ static void adv7511_mode_set(struct adv7511 *adv7511, > > vsync_polarity = 1; > > } > > > > - if (mode->vrefresh <= 24000) > > + if (drm_mode_vrefresh(mode) <= 24) > > low_refresh_rate = ADV7511_LOW_REFRESH_RATE_24HZ; > > - else if (mode->vrefresh <= 25000) > > + else if (drm_mode_vrefresh(mode) <= 25) > > low_refresh_rate = ADV7511_LOW_REFRESH_RATE_25HZ; > > - else if (mode->vrefresh <= 30000) > > + else if (drm_mode_vrefresh(mode) <= 30) > > low_refresh_rate = ADV7511_LOW_REFRESH_RATE_30HZ; > > else > > low_refresh_rate = ADV7511_LOW_REFRESH_RATE_NONE; > > -- > Regards, > > Laurent Pinchart > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS