Hi,
On Wed, Dec 05, 2018 at 05:00:17PM +0200, Laurent Pinchart wrote:
> The encoder .atomic_check() and connector .mode_valid() operations both
> walk through the dss devices in the pipeline to validate the mode.
> Factor out the common code in a new omap_drm_connector_mode_fixup()
> function.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
This is a bit tricky to review. It would probably be easier to
review, if the changes were split into two commits:
1. introduce common function
2. move drm_display_mode/videomode conversion further up the stack
Anyways:
Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
-- Sebastian
> drivers/gpu/drm/omapdrm/omap_connector.c | 55 +++++++++++++-----------
> drivers/gpu/drm/omapdrm/omap_connector.h | 5 +++
> drivers/gpu/drm/omapdrm/omap_encoder.c | 30 ++++---------
> 3 files changed, 45 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index dd1e0f2e8ffc..9882a4b1402b 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -241,50 +241,57 @@ static int omap_connector_get_modes(struct drm_connector *connector)
> return 0;
> }
>
> -static int omap_connector_mode_valid(struct drm_connector *connector,
> - struct drm_display_mode *mode)
> +enum drm_mode_status omap_connector_mode_fixup(struct omap_dss_device *dssdev,
> + const struct drm_display_mode *mode,
> + struct drm_display_mode *adjusted_mode)
> {
> - struct omap_connector *omap_connector = to_omap_connector(connector);
> - struct omap_dss_device *dssdev;
> - struct videomode vm = {0};
> - struct drm_device *dev = connector->dev;
> - struct drm_display_mode *new_mode;
> - int r, ret = MODE_BAD;
> + struct videomode vm = { 0 };
> + int ret;
>
> drm_display_mode_to_videomode(mode, &vm);
> - mode->vrefresh = drm_mode_vrefresh(mode);
>
> - for (dssdev = omap_connector->output; dssdev; dssdev = dssdev->next) {
> + for (; dssdev; dssdev = dssdev->next) {
> if (!dssdev->ops->check_timings)
> continue;
>
> - r = dssdev->ops->check_timings(dssdev, &vm);
> - if (r)
> - goto done;
> + ret = dssdev->ops->check_timings(dssdev, &vm);
> + if (ret)
> + return MODE_BAD;
> }
>
> - /* check if vrefresh is still valid */
> - new_mode = drm_mode_duplicate(dev, mode);
> - if (!new_mode)
> - return MODE_BAD;
> + if (adjusted_mode)
> + drm_display_mode_from_videomode(&vm, adjusted_mode);
>
> - new_mode->clock = vm.pixelclock / 1000;
> - new_mode->vrefresh = 0;
> - if (mode->vrefresh == drm_mode_vrefresh(new_mode))
> - ret = MODE_OK;
> - drm_mode_destroy(dev, new_mode);
> + return MODE_OK;
> +}
> +
> +static enum drm_mode_status omap_connector_mode_valid(struct drm_connector *connector,
> + struct drm_display_mode *mode)
> +{
> + struct omap_connector *omap_connector = to_omap_connector(connector);
> + struct drm_display_mode new_mode = { { 0 } };
> + enum drm_mode_status status;
> +
> + status = omap_connector_mode_fixup(omap_connector->output, mode,
> + &new_mode);
> + if (status != MODE_OK)
> + goto done;
> +
> + /* Check if vrefresh is still valid. */
> + if (drm_mode_vrefresh(mode) != drm_mode_vrefresh(&new_mode))
> + status = MODE_NOCLOCK;
>
> done:
> DBG("connector: mode %s: "
> "%d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x",
> - (ret == MODE_OK) ? "valid" : "invalid",
> + (status == MODE_OK) ? "valid" : "invalid",
> mode->base.id, mode->name, mode->vrefresh, mode->clock,
> mode->hdisplay, mode->hsync_start,
> mode->hsync_end, mode->htotal,
> mode->vdisplay, mode->vsync_start,
> mode->vsync_end, mode->vtotal, mode->type, mode->flags);
>
> - return ret;
> + return status;
> }
>
> static const struct drm_connector_funcs omap_connector_funcs = {
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.h b/drivers/gpu/drm/omapdrm/omap_connector.h
> index 4a1dcd0f031b..6b7d4d95e32b 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.h
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.h
> @@ -22,6 +22,8 @@
>
> #include <linux/types.h>
>
> +enum drm_mode_status;
> +
> struct drm_connector;
> struct drm_device;
> struct drm_encoder;
> @@ -34,5 +36,8 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
> bool omap_connector_get_hdmi_mode(struct drm_connector *connector);
> void omap_connector_enable_hpd(struct drm_connector *connector);
> void omap_connector_disable_hpd(struct drm_connector *connector);
> +enum drm_mode_status omap_connector_mode_fixup(struct omap_dss_device *dssdev,
> + const struct drm_display_mode *mode,
> + struct drm_display_mode *adjusted_mode);
>
> #endif /* __OMAPDRM_CONNECTOR_H__ */
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index 7d01df6fa723..086963088201 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -198,29 +198,17 @@ static int omap_encoder_atomic_check(struct drm_encoder *encoder,
> struct drm_connector_state *conn_state)
> {
> struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
> - struct drm_device *dev = encoder->dev;
> - struct omap_dss_device *dssdev;
> - struct videomode vm = { 0 };
> - int ret;
> -
> - drm_display_mode_to_videomode(&crtc_state->mode, &vm);
> -
> - for (dssdev = omap_encoder->output; dssdev; dssdev = dssdev->next) {
> - if (!dssdev->ops->check_timings)
> - continue;
> -
> - ret = dssdev->ops->check_timings(dssdev, &vm);
> - if (ret)
> - goto done;
> + enum drm_mode_status status;
> +
> + status = omap_connector_mode_fixup(omap_encoder->output,
> + &crtc_state->mode,
> + &crtc_state->adjusted_mode);
> + if (status != MODE_OK) {
> + dev_err(encoder->dev->dev, "invalid timings: %d\n", status);
> + return -EINVAL;
> }
>
> - drm_display_mode_from_videomode(&vm, &crtc_state->adjusted_mode);
> -
> -done:
> - if (ret)
> - dev_err(dev->dev, "invalid timings: %d\n", ret);
> -
> - return ret;
> + return 0;
> }
>
> static const struct drm_encoder_helper_funcs omap_encoder_helper_funcs = {
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
