| Message ID | 1504694220-15818-8-git-send-email-a.hajda@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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@math.uni-bielefeld.de> With one suggestion below. > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> > --- > 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? > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@math.uni-bielefeld.de> > > With one suggestion below. > > >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> >> --- >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); int (*atomic_check)(struct exynos_drm_crtc *crtc, struct drm_crtc_state *state); void (*atomic_begin)(struct exynos_drm_crtc *crtc);
crtc::mode_fixup callback is required by crtcs which use internally different mode than requested by user - case of Exynos Mixer. Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> --- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 15 +++++++++++++++ drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 +++ 2 files changed, 18 insertions(+)