From: Sam Ravnborg <sam@ravnborg.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: airlied@linux.ie, dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org, kraxel@redhat.com,
alexander.deucher@amd.com, spice-devel@lists.freedesktop.org,
emil.velikov@collabora.com
Subject: Re: [PATCH v3 3/4] drm/mgag200: Use simple encoder
Date: Thu, 27 Feb 2020 21:42:46 +0100 [thread overview]
Message-ID: <20200227204245.GE27592@ravnborg.org> (raw)
In-Reply-To: <20200225131055.27550-4-tzimmermann@suse.de>
On Tue, Feb 25, 2020 at 02:10:54PM +0100, Thomas Zimmermann wrote:
> The mgag200 driver uses an empty implementation for its encoder. Replace
> the code with the generic simple encoder.
>
> v3:
> * init pre-allocated encoder with drm_simple_encoder_init()
> v2:
> * rebase onto new simple-encoder interface
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/mgag200/mgag200_drv.h | 9 +--
> drivers/gpu/drm/mgag200/mgag200_mode.c | 85 +++-----------------------
> 2 files changed, 12 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index aa32aad222c2..9691252d6233 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -95,7 +95,6 @@
> #define MATROX_DPMS_CLEARED (-1)
>
> #define to_mga_crtc(x) container_of(x, struct mga_crtc, base)
> -#define to_mga_encoder(x) container_of(x, struct mga_encoder, base)
> #define to_mga_connector(x) container_of(x, struct mga_connector, base)
>
> struct mga_crtc {
> @@ -110,12 +109,6 @@ struct mga_mode_info {
> struct mga_crtc *crtc;
> };
>
> -struct mga_encoder {
> - struct drm_encoder base;
> - int last_dpms;
> -};
> -
> -
> struct mga_i2c_chan {
> struct i2c_adapter adapter;
> struct drm_device *dev;
> @@ -185,6 +178,8 @@ struct mga_device {
>
> /* SE model number stored in reg 0x1e24 */
> u32 unique_rev_id;
> +
> + struct drm_encoder encoder;
> };
>
> static inline enum mga_type
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 62a8e9ccb16d..efc9eaa6a4d4 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -15,6 +15,7 @@
> #include <drm/drm_fourcc.h>
> #include <drm/drm_plane_helper.h>
> #include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
>
> #include "mgag200_drv.h"
>
> @@ -1449,76 +1450,6 @@ static void mga_crtc_init(struct mga_device *mdev)
> drm_crtc_helper_add(&mga_crtc->base, &mga_helper_funcs);
> }
>
> -/*
> - * The encoder comes after the CRTC in the output pipeline, but before
> - * the connector. It's responsible for ensuring that the digital
> - * stream is appropriately converted into the output format. Setup is
> - * very simple in this case - all we have to do is inform qemu of the
> - * colour depth in order to ensure that it displays appropriately
> - */
> -
> -/*
> - * These functions are analagous to those in the CRTC code, but are intended
> - * to handle any encoder-specific limitations
> - */
> -static void mga_encoder_mode_set(struct drm_encoder *encoder,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> -{
> -
> -}
> -
> -static void mga_encoder_dpms(struct drm_encoder *encoder, int state)
> -{
> - return;
> -}
> -
> -static void mga_encoder_prepare(struct drm_encoder *encoder)
> -{
> -}
> -
> -static void mga_encoder_commit(struct drm_encoder *encoder)
> -{
> -}
> -
> -static void mga_encoder_destroy(struct drm_encoder *encoder)
> -{
> - struct mga_encoder *mga_encoder = to_mga_encoder(encoder);
> - drm_encoder_cleanup(encoder);
> - kfree(mga_encoder);
> -}
> -
> -static const struct drm_encoder_helper_funcs mga_encoder_helper_funcs = {
> - .dpms = mga_encoder_dpms,
> - .mode_set = mga_encoder_mode_set,
> - .prepare = mga_encoder_prepare,
> - .commit = mga_encoder_commit,
> -};
> -
> -static const struct drm_encoder_funcs mga_encoder_encoder_funcs = {
> - .destroy = mga_encoder_destroy,
> -};
> -
> -static struct drm_encoder *mga_encoder_init(struct drm_device *dev)
> -{
> - struct drm_encoder *encoder;
> - struct mga_encoder *mga_encoder;
> -
> - mga_encoder = kzalloc(sizeof(struct mga_encoder), GFP_KERNEL);
> - if (!mga_encoder)
> - return NULL;
> -
> - encoder = &mga_encoder->base;
> - encoder->possible_crtcs = 0x1;
> -
> - drm_encoder_init(dev, encoder, &mga_encoder_encoder_funcs,
> - DRM_MODE_ENCODER_DAC, NULL);
> - drm_encoder_helper_add(encoder, &mga_encoder_helper_funcs);
> -
> - return encoder;
> -}
> -
> -
> static int mga_vga_get_modes(struct drm_connector *connector)
> {
> struct mga_connector *mga_connector = to_mga_connector(connector);
> @@ -1686,8 +1617,9 @@ static struct drm_connector *mga_vga_init(struct drm_device *dev)
>
> int mgag200_modeset_init(struct mga_device *mdev)
> {
> - struct drm_encoder *encoder;
> + struct drm_encoder *encoder = &mdev->encoder;
> struct drm_connector *connector;
> + int ret;
>
> mdev->mode_info.mode_config_initialized = true;
>
> @@ -1698,11 +1630,14 @@ int mgag200_modeset_init(struct mga_device *mdev)
>
> mga_crtc_init(mdev);
>
> - encoder = mga_encoder_init(mdev->dev);
> - if (!encoder) {
> - DRM_ERROR("mga_encoder_init failed\n");
> - return -1;
> + ret = drm_simple_encoder_init(mdev->dev, encoder,
> + DRM_MODE_ENCODER_DAC);
> + if (ret) {
> + DRM_ERROR("drm_simple_encoder_init() failed, error %d\n",
> + -ret);
DRM_ERROR is deprecated if you have a drm_device.
Consider to use:
drm_err(mdev->dev, "drm_simple_encoder_init() failed, error %d\n",
ret);
Note - "-ret" looked strange. We usually do not modify return values
like this when printing.
> + return ret;
> }
> + encoder->possible_crtcs = 0x1;
>
> connector = mga_vga_init(mdev->dev);
> if (!connector) {
With the above addressed:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-02-27 20:42 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-25 13:10 [PATCH v3 0/4] drm: Provide a simple encoder Thomas Zimmermann
2020-02-25 13:10 ` [PATCH v3 1/4] drm/simple-kms: Add drm_simple_encoder_{init, create}() Thomas Zimmermann
2020-02-27 20:38 ` Sam Ravnborg
2020-02-25 13:10 ` [PATCH v3 2/4] drm/ast: Use simple encoder Thomas Zimmermann
2020-02-25 13:10 ` [PATCH v3 3/4] drm/mgag200: " Thomas Zimmermann
2020-02-27 20:42 ` Sam Ravnborg [this message]
2020-02-25 13:10 ` [PATCH v3 4/4] drm/qxl: " Thomas Zimmermann
2020-02-27 20:45 ` Sam Ravnborg
2020-02-28 7:08 ` Thomas Zimmermann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200227204245.GE27592@ravnborg.org \
--to=sam@ravnborg.org \
--cc=airlied@linux.ie \
--cc=alexander.deucher@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=emil.velikov@collabora.com \
--cc=kraxel@redhat.com \
--cc=spice-devel@lists.freedesktop.org \
--cc=tzimmermann@suse.de \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).