From: Thomas Zimmermann <tzimmermann@suse.de>
To: Daniel Vetter <daniel@ffwll.ch>, Gerd Hoffmann <kraxel@redhat.com>
Cc: airlied@redhat.com, chen@aspeedtech.com, sam@ravnborg.org,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 9/9] drm/ast: Enable atomic modesetting
Date: Wed, 6 Nov 2019 09:28:06 +0100 [thread overview]
Message-ID: <06c3f614-b0a8-83c6-c3c8-0b2a4451338f@suse.de> (raw)
In-Reply-To: <20191105103156.GD10326@phenom.ffwll.local>
[-- Attachment #1.1.1: Type: text/plain, Size: 13337 bytes --]
Hi
Am 05.11.19 um 11:31 schrieb Daniel Vetter:
> On Tue, Nov 05, 2019 at 10:57:11AM +0100, Gerd Hoffmann wrote:
>> On Mon, Oct 28, 2019 at 04:49:28PM +0100, Thomas Zimmermann wrote:
>>> This commit sets the remaining atomic-modesetting helpers and the flag
>>> DRIVER_ATOMIC. Legacy cursor functions are removed in favor of the cursor
>>> plane. For power management, atomic helpers replace the indvidual
>>> operations that the driver currently runs.
>>>
>>> Atomic modesetting is enabled with this commit.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>> drivers/gpu/drm/ast/ast_drv.c | 24 ++-
>>> drivers/gpu/drm/ast/ast_main.c | 5 +
>>> drivers/gpu/drm/ast/ast_mode.c | 290 ++-------------------------------
>>> 3 files changed, 33 insertions(+), 286 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
>>> index 1f17794b0890..d763da6f0834 100644
>>> --- a/drivers/gpu/drm/ast/ast_drv.c
>>> +++ b/drivers/gpu/drm/ast/ast_drv.c
>>> @@ -99,14 +99,14 @@ ast_pci_remove(struct pci_dev *pdev)
>>> drm_put_dev(dev);
>>> }
>>>
>>> -
>>> -
>>> static int ast_drm_freeze(struct drm_device *dev)
>>> {
>>> - drm_kms_helper_poll_disable(dev);
>>> - pci_save_state(dev->pdev);
>>> - drm_fb_helper_set_suspend_unlocked(dev->fb_helper, true);
>>> + int error;
>>>
>>> + error = drm_mode_config_helper_suspend(dev);
>>> + if (error)
>>> + return error;
>>> + pci_save_state(dev->pdev);
>>> return 0;
>>> }
>>>
>>> @@ -114,11 +114,7 @@ static int ast_drm_thaw(struct drm_device *dev)
>>> {
>>> ast_post_gpu(dev);
>>>
>>> - drm_mode_config_reset(dev);
>>> - drm_helper_resume_force_mode(dev);
>>> - drm_fb_helper_set_suspend_unlocked(dev->fb_helper, false);
>>> -
>>> - return 0;
>>> + return drm_mode_config_helper_resume(dev);
>>> }
>>>
>>> static int ast_drm_resume(struct drm_device *dev)
>>> @@ -131,8 +127,6 @@ static int ast_drm_resume(struct drm_device *dev)
>>> ret = ast_drm_thaw(dev);
>>> if (ret)
>>> return ret;
>>> -
>>> - drm_kms_helper_poll_enable(dev);
>>> return 0;
>>> }
>>>
>>> @@ -150,6 +144,7 @@ static int ast_pm_suspend(struct device *dev)
>>> pci_set_power_state(pdev, PCI_D3hot);
>>> return 0;
>>> }
>>> +
>>> static int ast_pm_resume(struct device *dev)
>>> {
>>> struct pci_dev *pdev = to_pci_dev(dev);
>>> @@ -165,7 +160,6 @@ static int ast_pm_freeze(struct device *dev)
>>> if (!ddev || !ddev->dev_private)
>>> return -ENODEV;
>>> return ast_drm_freeze(ddev);
>>> -
>>> }
>>>
>>> static int ast_pm_thaw(struct device *dev)
>>> @@ -203,7 +197,9 @@ static struct pci_driver ast_pci_driver = {
>>> DEFINE_DRM_GEM_FOPS(ast_fops);
>>>
>>> static struct drm_driver driver = {
>>> - .driver_features = DRIVER_MODESET | DRIVER_GEM,
>>> + .driver_features = DRIVER_ATOMIC |
>>> + DRIVER_GEM |
>>> + DRIVER_MODESET,
>>>
>>> .load = ast_driver_load,
>>> .unload = ast_driver_unload,
>>> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
>>> index 48d57ab42955..b79f484e9bd2 100644
>>> --- a/drivers/gpu/drm/ast/ast_main.c
>>> +++ b/drivers/gpu/drm/ast/ast_main.c
>>> @@ -28,6 +28,7 @@
>>>
>>> #include <linux/pci.h>
>>>
>>> +#include <drm/drm_atomic_helper.h>
>>> #include <drm/drm_crtc_helper.h>
>>> #include <drm/drm_fb_helper.h>
>>> #include <drm/drm_gem.h>
>>> @@ -412,6 +413,8 @@ enum drm_mode_status ast_mode_config_mode_valid(struct drm_device *dev,
>>> static const struct drm_mode_config_funcs ast_mode_funcs = {
>>> .fb_create = drm_gem_fb_create,
>>> .mode_valid = ast_mode_config_mode_valid,
>>> + .atomic_check = drm_atomic_helper_check,
>>> + .atomic_commit = drm_atomic_helper_commit,
>>> };
>>>
>>> static u32 ast_get_vram_info(struct drm_device *dev)
>>> @@ -529,6 +532,8 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>>> if (ret)
>>> goto out_free;
>>>
>>> + drm_mode_config_reset(dev);
>>> +
>>> ret = drm_fbdev_generic_setup(dev, 32);
>>> if (ret)
>>> goto out_free;
>>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>>> index f5f73200e8e4..5eccb6ae2ede 100644
>>> --- a/drivers/gpu/drm/ast/ast_mode.c
>>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>>> @@ -45,11 +45,6 @@
>>>
>>> static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
>>> static void ast_i2c_destroy(struct ast_i2c_chan *i2c);
>>> -static int ast_cursor_set(struct drm_crtc *crtc,
>>> - struct drm_file *file_priv,
>>> - uint32_t handle,
>>> - uint32_t width,
>>> - uint32_t height);
>>> static int ast_cursor_move(struct drm_crtc *crtc,
>>> int x, int y);
>>>
>>> @@ -58,9 +53,6 @@ static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
>>> static int ast_cursor_update(void *dst, void *src, unsigned int width,
>>> unsigned int height);
>>> static void ast_cursor_set_base(struct ast_private *ast, u64 address);
>>> -static int ast_show_cursor(struct drm_crtc *crtc, void *src,
>>> - unsigned int width, unsigned int height);
>>> -static void ast_hide_cursor(struct drm_crtc *crtc);
>>> static int ast_cursor_move(struct drm_crtc *crtc,
>>> int x, int y);
>>>
>>> @@ -434,7 +426,7 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct drm_display_mode *mod
>>> static void ast_set_offset_reg(struct drm_crtc *crtc)
>>> {
>>> struct ast_private *ast = crtc->dev->dev_private;
>>> - const struct drm_framebuffer *fb = crtc->primary->fb;
>>> + const struct drm_framebuffer *fb = crtc->primary->state->fb;
>>>
>>> u16 offset;
>>>
>>> @@ -528,7 +520,7 @@ static void ast_set_sync_reg(struct drm_device *dev, struct drm_display_mode *mo
>>> static bool ast_set_dac_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
>>> struct ast_vbios_mode_info *vbios_mode)
>>> {
>>> - const struct drm_framebuffer *fb = crtc->primary->fb;
>>> + const struct drm_framebuffer *fb = crtc->primary->state->fb;
>>>
>>> switch (fb->format->cpp[0] * 8) {
>>> case 8:
>>> @@ -765,112 +757,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
>>> }
>>> }
>>>
>>> -static int ast_crtc_do_set_base(struct drm_crtc *crtc,
>>> - struct drm_framebuffer *fb,
>>> - int x, int y, int atomic)
>>> -{
>>> - struct drm_gem_vram_object *gbo;
>>> - int ret;
>>> - s64 gpu_addr;
>>> -
>>> - if (!atomic && fb) {
>>> - gbo = drm_gem_vram_of_gem(fb->obj[0]);
>>> - drm_gem_vram_unpin(gbo);
>>> - }
>>> -
>>> - gbo = drm_gem_vram_of_gem(crtc->primary->fb->obj[0]);
>>> -
>>> - ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
>>> - if (ret)
>>> - return ret;
>>> - gpu_addr = drm_gem_vram_offset(gbo);
>>> - if (gpu_addr < 0) {
>>> - ret = (int)gpu_addr;
>>> - goto err_drm_gem_vram_unpin;
>>> - }
>>> -
>>> - ast_set_offset_reg(crtc);
>>> - ast_set_start_address_crt1(crtc, (u32)gpu_addr);
>>> -
>>> - return 0;
>>> -
>>> -err_drm_gem_vram_unpin:
>>> - drm_gem_vram_unpin(gbo);
>>> - return ret;
>>> -}
>>> -
>>> -static int ast_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>>> - struct drm_framebuffer *old_fb)
>>> -{
>>> - return ast_crtc_do_set_base(crtc, old_fb, x, y, 0);
>>> -}
>>> -
>>> -static int ast_crtc_mode_set(struct drm_crtc *crtc,
>>> - struct drm_display_mode *mode,
>>> - struct drm_display_mode *adjusted_mode,
>>> - int x, int y,
>>> - struct drm_framebuffer *old_fb)
>>> -{
>>> - struct drm_device *dev = crtc->dev;
>>> - struct ast_private *ast = crtc->dev->dev_private;
>>> - const struct drm_framebuffer *fb = crtc->primary->fb;
>>> - struct ast_vbios_mode_info vbios_mode;
>>> - bool succ;
>>> -
>>> - if (ast->chip == AST1180) {
>>> - DRM_ERROR("AST 1180 modesetting not supported\n");
>>> - return -EINVAL;
>>> - }
>>> -
>>> - succ = ast_get_vbios_mode_info(fb, mode, adjusted_mode, &vbios_mode);
>>> - if (!succ)
>>> - return -EINVAL;
>>> -
>>> - ast_open_key(ast);
>>> -
>>> - ast_set_vbios_color_reg(crtc, fb, &vbios_mode);
>>> - ast_set_vbios_mode_reg(crtc, adjusted_mode, &vbios_mode);
>>> - ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
>>> - ast_set_std_reg(crtc, adjusted_mode, &vbios_mode);
>>> - ast_set_crtc_reg(crtc, adjusted_mode, &vbios_mode);
>>> - ast_set_offset_reg(crtc);
>>> - ast_set_dclk_reg(dev, adjusted_mode, &vbios_mode);
>>> - ast_set_color_reg(crtc, fb);
>>> - ast_set_crtthd_reg(crtc);
>>> - ast_set_sync_reg(dev, adjusted_mode, &vbios_mode);
>>> - ast_set_dac_reg(crtc, adjusted_mode, &vbios_mode);
>>> -
>>> - ast_crtc_mode_set_base(crtc, x, y, old_fb);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static void ast_crtc_disable(struct drm_crtc *crtc)
>>> -{
>>> - DRM_DEBUG_KMS("\n");
>>> - ast_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
>>> - if (crtc->primary->fb) {
>>> - struct drm_framebuffer *fb = crtc->primary->fb;
>>> - struct drm_gem_vram_object *gbo =
>>> - drm_gem_vram_of_gem(fb->obj[0]);
>>> -
>>> - drm_gem_vram_unpin(gbo);
>>> - }
>>> - crtc->primary->fb = NULL;
>>> -}
>>> -
>>> -static void ast_crtc_prepare(struct drm_crtc *crtc)
>>> -{
>>> -
>>> -}
>>> -
>>> -static void ast_crtc_commit(struct drm_crtc *crtc)
>>> -{
>>> - struct ast_private *ast = crtc->dev->dev_private;
>>> - ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0);
>>> - ast_crtc_load_lut(crtc);
>>> -}
>>> -
>>> static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
>>> struct drm_crtc_state *state)
>>> {
>>> @@ -970,12 +856,6 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
>>> }
>>>
>>> static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>>> - .dpms = ast_crtc_dpms,
>>> - .mode_set = ast_crtc_mode_set,
>>> - .mode_set_base = ast_crtc_mode_set_base,
>>> - .disable = ast_crtc_disable,
>>> - .prepare = ast_crtc_prepare,
>>> - .commit = ast_crtc_commit,
>>> .atomic_check = ast_crtc_helper_atomic_check,
>>> .atomic_begin = ast_crtc_helper_atomic_begin,
>>> .atomic_flush = ast_crtc_helper_atomic_flush,
>>> @@ -983,21 +863,6 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
>>> .atomic_disable = ast_crtc_helper_atomic_disable,
>>> };
>>>
>>> -static void ast_crtc_reset(struct drm_crtc *crtc)
>>> -{
>>> -
>>> -}
>>> -
>>> -static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>>> - u16 *blue, uint32_t size,
>>> - struct drm_modeset_acquire_ctx *ctx)
>>> -{
>>> - ast_crtc_load_lut(crtc);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -
>>> static void ast_crtc_destroy(struct drm_crtc *crtc)
>>> {
>>> drm_crtc_cleanup(crtc);
>>> @@ -1005,12 +870,12 @@ static void ast_crtc_destroy(struct drm_crtc *crtc)
>>> }
>>>
>>> static const struct drm_crtc_funcs ast_crtc_funcs = {
>>> - .cursor_set = ast_cursor_set,
>>> - .cursor_move = ast_cursor_move,
>>> - .reset = ast_crtc_reset,
>>> + .reset = drm_atomic_helper_crtc_reset,
>>> .set_config = drm_crtc_helper_set_config,
>>> - .gamma_set = ast_crtc_gamma_set,
>>> + .gamma_set = drm_atomic_helper_legacy_gamma_set,
>>> .destroy = ast_crtc_destroy,
>>> + .set_config = drm_atomic_helper_set_config,
>>> + .page_flip = drm_atomic_helper_page_flip,
>>> .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>>> .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>>> };
>>> @@ -1040,6 +905,10 @@ static int ast_crtc_init(struct drm_device *dev)
>>> return ret;
>>> }
>>>
>>> +/*
>>> + * Encoder
>>> + */
>>> +
>>> static void ast_encoder_destroy(struct drm_encoder *encoder)
>>> {
>>> drm_encoder_cleanup(encoder);
>>> @@ -1050,34 +919,6 @@ static const struct drm_encoder_funcs ast_enc_funcs = {
>>> .destroy = ast_encoder_destroy,
>>> };
>>>
>>> -static void ast_encoder_dpms(struct drm_encoder *encoder, int mode)
>>> -{
>>> -
>>> -}
>>> -
>>> -static void ast_encoder_mode_set(struct drm_encoder *encoder,
>>> - struct drm_display_mode *mode,
>>> - struct drm_display_mode *adjusted_mode)
>>> -{
>>> -}
>>> -
>>> -static void ast_encoder_prepare(struct drm_encoder *encoder)
>>> -{
>>> -
>>> -}
>>> -
>>> -static void ast_encoder_commit(struct drm_encoder *encoder)
>>> -{
>>> -
>>> -}
>>> -
>>> -static const struct drm_encoder_helper_funcs ast_enc_helper_funcs = {
>>> - .dpms = ast_encoder_dpms,
>>> - .prepare = ast_encoder_prepare,
>>> - .commit = ast_encoder_commit,
>>> - .mode_set = ast_encoder_mode_set,
>>> -};
>>
>> Hmm. Pretty much a dummy encoder implementation. Maybe ast is simple
>> enough that the simple pipe helpers can do the trick?
>
> simple pipe doesn't do multi-plane, so not applicable. Maybe there's a few
> more things we can for dummy encoders (like default cleanup perhaps?).
I'll see what I can do for the patchset's next iteration.
Best regards
Thomas
> -Daniel
>
>>
>> cheers,
>> Gerd
>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-11-06 8:28 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-28 15:49 [PATCH 0/9] drm/ast: Convert to atomic modesetting Thomas Zimmermann
2019-10-28 15:49 ` [PATCH 1/9] drm/ast: Remove last traces of struct ast_gem_object Thomas Zimmermann
2019-11-05 9:42 ` Gerd Hoffmann
2019-10-28 15:49 ` [PATCH 2/9] drm/ast: Check video-mode requirements against VRAM size Thomas Zimmermann
2019-11-05 9:42 ` Gerd Hoffmann
2019-10-28 15:49 ` [PATCH 3/9] drm/ast: Don't clear base address and offset with default values Thomas Zimmermann
2019-11-05 9:44 ` Gerd Hoffmann
2019-10-28 15:49 ` [PATCH 4/9] drm/ast: Split ast_set_ext_reg() into color and threshold function Thomas Zimmermann
2019-11-05 9:45 ` Gerd Hoffmann
2019-10-28 15:49 ` [PATCH 5/9] drm/ast: Split ast_set_vbios_mode_info() Thomas Zimmermann
2019-11-05 9:47 ` Gerd Hoffmann
2019-10-28 15:49 ` [PATCH 6/9] drm/ast: Add primary plane Thomas Zimmermann
2019-11-05 9:51 ` Gerd Hoffmann
2019-11-05 9:54 ` Daniel Vetter
2019-11-06 8:24 ` Thomas Zimmermann
2019-10-28 15:49 ` [PATCH 7/9] drm/ast: Add CRTC helpers for atomic modesetting Thomas Zimmermann
2019-11-05 9:51 ` Gerd Hoffmann
2019-10-28 15:49 ` [PATCH 8/9] drm/ast: Add cursor plane Thomas Zimmermann
2019-11-05 9:52 ` Gerd Hoffmann
2019-11-05 9:55 ` Daniel Vetter
2019-11-06 8:31 ` Thomas Zimmermann
2019-11-06 9:05 ` Daniel Vetter
2019-11-06 9:46 ` Thomas Zimmermann
2019-10-28 15:49 ` [PATCH 9/9] drm/ast: Enable atomic modesetting Thomas Zimmermann
2019-11-05 9:57 ` Gerd Hoffmann
2019-11-05 10:31 ` Daniel Vetter
2019-11-06 8:28 ` Thomas Zimmermann [this message]
2019-11-06 13:36 ` Thomas Zimmermann
2019-11-07 6:55 ` Gerd Hoffmann
2019-11-07 7:32 ` Thomas Zimmermann
2019-10-28 16:00 ` [PATCH 0/9] drm/ast: Convert to " Thomas Zimmermann
2019-10-28 16:00 ` 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=06c3f614-b0a8-83c6-c3c8-0b2a4451338f@suse.de \
--to=tzimmermann@suse.de \
--cc=airlied@redhat.com \
--cc=chen@aspeedtech.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=kraxel@redhat.com \
--cc=sam@ravnborg.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.