On 12.09.2017 14:41, Tobias Jakobi wrote: > Hello Andrzej, > > > Andrzej Hajda wrote: >> crtc::mode_fixup callback is required by crtcs which use internally >> different mode than requested by user - case of Exynos Mixer. > "...which internally use a different..." > > > Reviewed-by: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx> > > With one suggestion below. > > >> Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx> >> --- >> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 15 +++++++++++++++ >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 +++ >> 2 files changed, 18 insertions(+) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> index 6ce0821..dc01342 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> @@ -95,8 +95,23 @@ static enum drm_mode_status exynos_crtc_mode_valid(struct drm_crtc *crtc, >> return MODE_OK; >> } >> >> +static bool exynos_crtc_mode_fixup(struct drm_crtc *crtc, >> + const struct drm_display_mode *mode, >> + struct drm_display_mode *adjusted_mode) >> +{ >> + struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc); >> + >> + if (exynos_crtc->ops->mode_fixup) >> + return exynos_crtc->ops->mode_fixup(exynos_crtc, mode, >> + adjusted_mode); >> + >> + return true; >> +} >> + >> + >> static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = { >> .mode_valid = exynos_crtc_mode_valid, >> + .mode_fixup = exynos_crtc_mode_fixup, >> .atomic_check = exynos_crtc_atomic_check, >> .atomic_begin = exynos_crtc_atomic_begin, >> .atomic_flush = exynos_crtc_atomic_flush, >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h >> index cf131c2..e8bcc72 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h >> @@ -136,6 +136,9 @@ struct exynos_drm_crtc_ops { >> u32 (*get_vblank_counter)(struct exynos_drm_crtc *crtc); >> enum drm_mode_status (*mode_valid)(struct exynos_drm_crtc *crtc, >> const struct drm_display_mode *mode); >> + bool (*mode_fixup)(struct exynos_drm_crtc *crtc, >> + const struct drm_display_mode *mode, >> + struct drm_display_mode *adjusted_mode); > I'm always wary when I encounter a bool as return value, since to check what > true/false actually encodes, I need to have some reference which I can check. > Just go for an integer here and use standard convention that negative values are > errors? It is forwarder for drm core op so I prefer to keep return values consistent. Regards Andrzej > > > >> int (*atomic_check)(struct exynos_drm_crtc *crtc, >> struct drm_crtc_state *state); >> void (*atomic_begin)(struct exynos_drm_crtc *crtc); >> > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html