| Message ID | 1455103276-2963-1-git-send-email-palminha@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 10, 2016 at 11:21:16AM +0000, Carlos Palminha wrote: > Avoid drivers to copy/past code to implement functions that will only return > true. > > Changes v1->v2: > * Added Signed off by > > Signed-off-by: Carlos Palminha <palminha@synopsys.com> > --- > drivers/gpu/drm/drm_bridge.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index bd93453..4133e51 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -161,6 +161,8 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge, > > if (bridge->funcs->mode_fixup) > ret = bridge->funcs->mode_fixup(bridge, mode, adjusted_mode); > + else > + return true; Ok, this doesn't make sense - if there's no fixup function for this bridge we should still call any fixup functions of bridges further down the chain. And ret is initialized already to true, so this should work. Where/how exactly do you see this blow up? Aside: dw-hdmi has a dummy mode_fixup function in it's bridge vtable, can you pls create a patch to remove that one? All other bridge drivers don't have a mode_fixup function at all. -Daniel > > ret = ret && drm_bridge_mode_fixup(bridge->next, mode, adjusted_mode); > > -- > 2.5.0 >
I'll send a v3 patch with the removal for dw-hdmi On 10-02-2016 12:22, Daniel Vetter wrote: > On Wed, Feb 10, 2016 at 11:21:16AM +0000, Carlos Palminha wrote: >> Avoid drivers to copy/past code to implement functions that will only return >> true. >> >> Changes v1->v2: >> * Added Signed off by >> >> Signed-off-by: Carlos Palminha <palminha@synopsys.com> >> --- >> drivers/gpu/drm/drm_bridge.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c >> index bd93453..4133e51 100644 >> --- a/drivers/gpu/drm/drm_bridge.c >> +++ b/drivers/gpu/drm/drm_bridge.c >> @@ -161,6 +161,8 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge, >> >> if (bridge->funcs->mode_fixup) >> ret = bridge->funcs->mode_fixup(bridge, mode, adjusted_mode); >> + else >> + return true; > > Ok, this doesn't make sense - if there's no fixup function for this bridge > we should still call any fixup functions of bridges further down the > chain. And ret is initialized already to true, so this should work. > > Where/how exactly do you see this blow up? > > Aside: dw-hdmi has a dummy mode_fixup function in it's bridge vtable, can > you pls create a patch to remove that one? All other bridge drivers don't > have a mode_fixup function at all. > -Daniel > >> >> ret = ret && drm_bridge_mode_fixup(bridge->next, mode, adjusted_mode); >> >> -- >> 2.5.0 >> >
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index bd93453..4133e51 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -161,6 +161,8 @@ bool drm_bridge_mode_fixup(struct drm_bridge *bridge, if (bridge->funcs->mode_fixup) ret = bridge->funcs->mode_fixup(bridge, mode, adjusted_mode); + else + return true; ret = ret && drm_bridge_mode_fixup(bridge->next, mode, adjusted_mode);
Avoid drivers to copy/past code to implement functions that will only return true. Changes v1->v2: * Added Signed off by Signed-off-by: Carlos Palminha <palminha@synopsys.com> --- drivers/gpu/drm/drm_bridge.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.5.0