On Fri, May 17, 2019 at 05:37:21PM +0100, Robin Murphy wrote: > Rather than allowing any old mode through, then subsequently refusing > unmatchable clock rates in atomic_check when it's too late to back out > and pick a different mode, let's do that validation up-front where it > will cause unsupported modes to be correctly pruned in the first place. > > This also eliminates an issue whereby a perceived clock rate of 0 would > cause atomic disable to fail and prevent the module from being unloaded. > > Signed-off-by: Robin Murphy <robin.mur...@arm.com>
Acked-by: Liviu Dudau <liviu.du...@arm.com> Thanks for the patch! Best regards, Liviu > --- > > This supersedes my previous patch here: > https://patchwork.freedesktop.org/patch/288553/ > --- > drivers/gpu/drm/arm/hdlcd_crtc.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c > b/drivers/gpu/drm/arm/hdlcd_crtc.c > index 0b2b62f8fa3c..ecac6fe0b213 100644 > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c > @@ -186,20 +186,19 @@ static void hdlcd_crtc_atomic_disable(struct drm_crtc > *crtc, > clk_disable_unprepare(hdlcd->clk); > } > > -static int hdlcd_crtc_atomic_check(struct drm_crtc *crtc, > - struct drm_crtc_state *state) > +static enum drm_mode_status hdlcd_crtc_mode_valid(struct drm_crtc *crtc, > + const struct drm_display_mode *mode) > { > struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc); > - struct drm_display_mode *mode = &state->adjusted_mode; > long rate, clk_rate = mode->clock * 1000; > > rate = clk_round_rate(hdlcd->clk, clk_rate); > if (rate != clk_rate) { > /* clock required by mode not supported by hardware */ > - return -EINVAL; > + return MODE_NOCLOCK; > } > > - return 0; > + return MODE_OK; > } > > static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc, > @@ -220,7 +219,7 @@ static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc, > } > > static const struct drm_crtc_helper_funcs hdlcd_crtc_helper_funcs = { > - .atomic_check = hdlcd_crtc_atomic_check, > + .mode_valid = hdlcd_crtc_mode_valid, > .atomic_begin = hdlcd_crtc_atomic_begin, > .atomic_enable = hdlcd_crtc_atomic_enable, > .atomic_disable = hdlcd_crtc_atomic_disable, > -- > 2.21.0.dirty > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯
