* [PATCH 0/3] ast cleanups
@ 2020-06-11 8:28 Thomas Zimmermann
2020-06-11 8:28 ` [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180 Thomas Zimmermann
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Thomas Zimmermann @ 2020-06-11 8:28 UTC (permalink / raw)
To: airlied, daniel, sam, emil.velikov, kraxel, noralf, chen
Cc: Thomas Zimmermann, dri-devel
Ast still has to be converted to managed initialization, and embed the
DRM device in the ast structure. In preparation of these changes, add
some cleanups to the driver.
Tested on ast HW.
Thomas Zimmermann (3):
drm/ast: Remove unused code paths for AST 1180
drm/ast: Upcast from DRM device to ast structure via to_ast_private()
drm/ast: Use per-device logging macros
drivers/gpu/drm/ast/ast_dp501.c | 24 +++----
drivers/gpu/drm/ast/ast_drv.c | 1 -
drivers/gpu/drm/ast/ast_drv.h | 7 +-
drivers/gpu/drm/ast/ast_main.c | 115 +++++++++++++++-----------------
drivers/gpu/drm/ast/ast_mode.c | 59 +++++++---------
drivers/gpu/drm/ast/ast_post.c | 28 ++++----
drivers/gpu/drm/ast/ast_ttm.c | 2 +-
7 files changed, 107 insertions(+), 129 deletions(-)
--
2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180
2020-06-11 8:28 [PATCH 0/3] ast cleanups Thomas Zimmermann
@ 2020-06-11 8:28 ` Thomas Zimmermann
2020-06-11 17:24 ` Daniel Vetter
2020-06-15 23:21 ` Emil Velikov
2020-06-11 8:28 ` [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private() Thomas Zimmermann
2020-06-11 8:28 ` [PATCH 3/3] drm/ast: Use per-device logging macros Thomas Zimmermann
2 siblings, 2 replies; 17+ messages in thread
From: Thomas Zimmermann @ 2020-06-11 8:28 UTC (permalink / raw)
To: airlied, daniel, sam, emil.velikov, kraxel, noralf, chen
Cc: Thomas Zimmermann, dri-devel
The ast driver contains code paths for AST 1180 chips. The chip is not
supported and the rsp code has never been tested. Simplify the driver by
removing the AST 1180 code.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/ast/ast_drv.c | 1 -
drivers/gpu/drm/ast/ast_drv.h | 2 -
drivers/gpu/drm/ast/ast_main.c | 89 +++++++++++++++-------------------
drivers/gpu/drm/ast/ast_mode.c | 10 +---
drivers/gpu/drm/ast/ast_post.c | 10 ++--
5 files changed, 43 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index b7ba22dddcad9..83509106f3ba9 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -59,7 +59,6 @@ static struct drm_driver driver;
static const struct pci_device_id pciidlist[] = {
AST_VGA_DEVICE(PCI_CHIP_AST2000, NULL),
AST_VGA_DEVICE(PCI_CHIP_AST2100, NULL),
- /* AST_VGA_DEVICE(PCI_CHIP_AST1180, NULL), - don't bind to 1180 for now */
{0, 0, 0},
};
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 656d591b154b3..09f2659e29118 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -52,7 +52,6 @@
#define PCI_CHIP_AST2000 0x2000
#define PCI_CHIP_AST2100 0x2010
-#define PCI_CHIP_AST1180 0x1180
enum ast_chip {
@@ -64,7 +63,6 @@ enum ast_chip {
AST2300,
AST2400,
AST2500,
- AST1180,
};
enum ast_tx_chip {
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index e5398e3dabe70..f48a9f62368c0 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -142,50 +142,42 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
ast_detect_config_mode(dev, &scu_rev);
/* Identify chipset */
- if (dev->pdev->device == PCI_CHIP_AST1180) {
- ast->chip = AST1100;
- DRM_INFO("AST 1180 detected\n");
- } else {
- if (dev->pdev->revision >= 0x40) {
- ast->chip = AST2500;
- DRM_INFO("AST 2500 detected\n");
- } else if (dev->pdev->revision >= 0x30) {
- ast->chip = AST2400;
- DRM_INFO("AST 2400 detected\n");
- } else if (dev->pdev->revision >= 0x20) {
- ast->chip = AST2300;
- DRM_INFO("AST 2300 detected\n");
- } else if (dev->pdev->revision >= 0x10) {
- switch (scu_rev & 0x0300) {
- case 0x0200:
- ast->chip = AST1100;
- DRM_INFO("AST 1100 detected\n");
- break;
- case 0x0100:
- ast->chip = AST2200;
- DRM_INFO("AST 2200 detected\n");
- break;
- case 0x0000:
- ast->chip = AST2150;
- DRM_INFO("AST 2150 detected\n");
- break;
- default:
- ast->chip = AST2100;
- DRM_INFO("AST 2100 detected\n");
- break;
- }
- ast->vga2_clone = false;
- } else {
- ast->chip = AST2000;
- DRM_INFO("AST 2000 detected\n");
+ if (dev->pdev->revision >= 0x40) {
+ ast->chip = AST2500;
+ DRM_INFO("AST 2500 detected\n");
+ } else if (dev->pdev->revision >= 0x30) {
+ ast->chip = AST2400;
+ DRM_INFO("AST 2400 detected\n");
+ } else if (dev->pdev->revision >= 0x20) {
+ ast->chip = AST2300;
+ DRM_INFO("AST 2300 detected\n");
+ } else if (dev->pdev->revision >= 0x10) {
+ switch (scu_rev & 0x0300) {
+ case 0x0200:
+ ast->chip = AST1100;
+ DRM_INFO("AST 1100 detected\n");
+ break;
+ case 0x0100:
+ ast->chip = AST2200;
+ DRM_INFO("AST 2200 detected\n");
+ break;
+ case 0x0000:
+ ast->chip = AST2150;
+ DRM_INFO("AST 2150 detected\n");
+ break;
+ default:
+ ast->chip = AST2100;
+ DRM_INFO("AST 2100 detected\n");
+ break;
}
+ ast->vga2_clone = false;
+ } else {
+ ast->chip = AST2000;
+ DRM_INFO("AST 2000 detected\n");
}
/* Check if we support wide screen */
switch (ast->chip) {
- case AST1180:
- ast->support_wide_screen = true;
- break;
case AST2000:
ast->support_wide_screen = false;
break;
@@ -469,15 +461,13 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
if (need_post)
ast_post_gpu(dev);
- if (ast->chip != AST1180) {
- ret = ast_get_dram_info(dev);
- if (ret)
- goto out_free;
- ast->vram_size = ast_get_vram_info(dev);
- DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
- ast->mclk, ast->dram_type,
- ast->dram_bus_width, ast->vram_size);
- }
+ ret = ast_get_dram_info(dev);
+ if (ret)
+ goto out_free;
+ ast->vram_size = ast_get_vram_info(dev);
+ DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
+ ast->mclk, ast->dram_type,
+ ast->dram_bus_width, ast->vram_size);
ret = ast_mm_init(ast);
if (ret)
@@ -496,8 +486,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
ast->chip == AST2200 ||
ast->chip == AST2300 ||
ast->chip == AST2400 ||
- ast->chip == AST2500 ||
- ast->chip == AST1180) {
+ ast->chip == AST2500) {
dev->mode_config.max_width = 1920;
dev->mode_config.max_height = 2048;
} else {
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 7d39b858c9f1f..be0e2250708fa 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -768,9 +768,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
{
struct ast_private *ast = crtc->dev->dev_private;
- if (ast->chip == AST1180)
- return;
-
/* TODO: Maybe control display signal generation with
* Sync Enable (bit CR17.7).
*/
@@ -797,11 +794,6 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
const struct drm_format_info *format;
bool succ;
- if (ast->chip == AST1180) {
- DRM_ERROR("AST 1180 modesetting not supported\n");
- return -EINVAL;
- }
-
if (!state->enable)
return 0; /* no mode checks if CRTC is being disabled */
@@ -1043,7 +1035,7 @@ static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
if ((ast->chip == AST2100) || (ast->chip == AST2200) ||
(ast->chip == AST2300) || (ast->chip == AST2400) ||
- (ast->chip == AST2500) || (ast->chip == AST1180)) {
+ (ast->chip == AST2500)) {
if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080))
return MODE_OK;
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 2d1b186197432..af0c8ebb009a1 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -58,13 +58,9 @@ bool ast_is_vga_enabled(struct drm_device *dev)
struct ast_private *ast = dev->dev_private;
u8 ch;
- if (ast->chip == AST1180) {
- /* TODO 1180 */
- } else {
- ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
- return !!(ch & 0x01);
- }
- return false;
+ ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
+
+ return !!(ch & 0x01);
}
static const u8 extreginfo[] = { 0x0f, 0x04, 0x1c, 0xff };
--
2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private()
2020-06-11 8:28 [PATCH 0/3] ast cleanups Thomas Zimmermann
2020-06-11 8:28 ` [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180 Thomas Zimmermann
@ 2020-06-11 8:28 ` Thomas Zimmermann
2020-06-11 17:32 ` Daniel Vetter
2020-06-11 19:23 ` Sam Ravnborg
2020-06-11 8:28 ` [PATCH 3/3] drm/ast: Use per-device logging macros Thomas Zimmermann
2 siblings, 2 replies; 17+ messages in thread
From: Thomas Zimmermann @ 2020-06-11 8:28 UTC (permalink / raw)
To: airlied, daniel, sam, emil.velikov, kraxel, noralf, chen
Cc: Thomas Zimmermann, dri-devel
All upcasting from struct drm_device to struct ast_private is now
performed via to_ast_private(). Using struct drm_device.dev_private is
deprecated. The ast variable in ast_crtc_helperatomic_check() is unused,
so removed it.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/ast/ast_dp501.c | 24 +++++++++----------
drivers/gpu/drm/ast/ast_drv.h | 5 ++++
drivers/gpu/drm/ast/ast_main.c | 10 ++++----
drivers/gpu/drm/ast/ast_mode.c | 41 ++++++++++++++++-----------------
drivers/gpu/drm/ast/ast_post.c | 16 ++++++-------
5 files changed, 50 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 98cd69269263f..4b85a504825a2 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -10,7 +10,7 @@ MODULE_FIRMWARE("ast_dp501_fw.bin");
static int ast_load_dp501_microcode(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
return request_firmware(&ast->dp501_fw, "ast_dp501_fw.bin", dev->dev);
}
@@ -93,7 +93,7 @@ static bool wait_fw_ready(struct ast_private *ast)
static bool ast_write_cmd(struct drm_device *dev, u8 data)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
int retry = 0;
if (wait_nack(ast)) {
send_nack(ast);
@@ -115,7 +115,7 @@ static bool ast_write_cmd(struct drm_device *dev, u8 data)
static bool ast_write_data(struct drm_device *dev, u8 data)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
if (wait_nack(ast)) {
send_nack(ast);
@@ -133,7 +133,7 @@ static bool ast_write_data(struct drm_device *dev, u8 data)
#if 0
static bool ast_read_data(struct drm_device *dev, u8 *data)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u8 tmp;
*data = 0;
@@ -172,7 +172,7 @@ static u32 get_fw_base(struct ast_private *ast)
bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u32 i, data;
u32 boot_address;
@@ -188,7 +188,7 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
static bool ast_launch_m68k(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u32 i, data, len = 0;
u32 boot_address;
u8 *fw_addr = NULL;
@@ -255,7 +255,7 @@ static bool ast_launch_m68k(struct drm_device *dev)
u8 ast_get_dp501_max_clk(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u32 boot_address, offset, data;
u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
@@ -283,7 +283,7 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u32 i, boot_address, offset, data;
boot_address = get_fw_base(ast);
@@ -312,7 +312,7 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
static bool ast_init_dvo(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u8 jreg;
u32 data;
ast_write32(ast, 0xf004, 0x1e6e0000);
@@ -385,7 +385,7 @@ static bool ast_init_dvo(struct drm_device *dev)
static void ast_init_analog(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u32 data;
/*
@@ -412,7 +412,7 @@ static void ast_init_analog(struct drm_device *dev)
void ast_init_3rdtx(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u8 jreg;
if (ast->chip == AST2300 || ast->chip == AST2400) {
@@ -438,7 +438,7 @@ void ast_init_3rdtx(struct drm_device *dev)
void ast_release_firmware(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
release_firmware(ast->dp501_fw);
ast->dp501_fw = NULL;
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 09f2659e29118..c44c1376c6977 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -136,6 +136,11 @@ struct ast_private {
const struct firmware *dp501_fw; /* dp501 fw */
};
+static inline struct ast_private *to_ast_private(struct drm_device *dev)
+{
+ return dev->dev_private;
+}
+
int ast_driver_load(struct drm_device *dev, unsigned long flags);
void ast_driver_unload(struct drm_device *dev);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f48a9f62368c0..a2ef3d9077671 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -67,7 +67,7 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
{
struct device_node *np = dev->pdev->dev.of_node;
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
uint32_t data, jregd0, jregd1;
/* Defaults */
@@ -117,7 +117,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
static int ast_detect_chip(struct drm_device *dev, bool *need_post)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
uint32_t jreg, scu_rev;
/*
@@ -262,7 +262,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
static int ast_get_dram_info(struct drm_device *dev)
{
struct device_node *np = dev->pdev->dev.of_node;
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
uint32_t denum, num, div, ref_pll, dsel;
@@ -388,7 +388,7 @@ static const struct drm_mode_config_funcs ast_mode_funcs = {
static u32 ast_get_vram_info(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u8 jreg;
u32 vram_size;
ast_open_key(ast);
@@ -509,7 +509,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
void ast_driver_unload(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
/* enable standard VGA decode */
ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index be0e2250708fa..10211751182da 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -565,7 +565,7 @@ static void
ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_plane_state *old_state)
{
- struct ast_private *ast = plane->dev->dev_private;
+ struct ast_private *ast = to_ast_private(plane->dev);
struct drm_plane_state *state = plane->state;
struct drm_gem_vram_object *gbo;
s64 gpu_addr;
@@ -585,7 +585,7 @@ static void
ast_primary_plane_helper_atomic_disable(struct drm_plane *plane,
struct drm_plane_state *old_state)
{
- struct ast_private *ast = plane->dev->dev_private;
+ struct ast_private *ast = to_ast_private(plane->dev);
ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x20);
}
@@ -633,7 +633,7 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
WARN_ON_ONCE(fb->height > AST_MAX_HWC_HEIGHT))
return -EINVAL; /* BUG: didn't test in atomic_check() */
- ast = crtc->dev->dev_private;
+ ast = to_ast_private(crtc->dev);
gbo = drm_gem_vram_of_gem(fb->obj[0]);
src = drm_gem_vram_vmap(gbo);
@@ -705,7 +705,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_plane_state *state = plane->state;
struct drm_crtc *crtc = state->crtc;
struct drm_framebuffer *fb = state->fb;
- struct ast_private *ast = plane->dev->dev_private;
+ struct ast_private *ast = to_ast_private(plane->dev);
struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
struct drm_gem_vram_object *gbo;
s64 off;
@@ -738,7 +738,7 @@ static void
ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
struct drm_plane_state *old_state)
{
- struct ast_private *ast = plane->dev->dev_private;
+ struct ast_private *ast = to_ast_private(plane->dev);
ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
}
@@ -766,7 +766,7 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs = {
static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
{
- struct ast_private *ast = crtc->dev->dev_private;
+ struct ast_private *ast = to_ast_private(crtc->dev);
/* TODO: Maybe control display signal generation with
* Sync Enable (bit CR17.7).
@@ -789,7 +789,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
- struct ast_private *ast = crtc->dev->dev_private;
struct ast_crtc_state *ast_state;
const struct drm_format_info *format;
bool succ;
@@ -815,7 +814,7 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
{
- struct ast_private *ast = crtc->dev->dev_private;
+ struct ast_private *ast = to_ast_private(crtc->dev);
ast_open_key(ast);
}
@@ -824,7 +823,7 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
{
struct drm_device *dev = crtc->dev;
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
struct ast_crtc_state *ast_state;
const struct drm_format_info *format;
struct ast_vbios_mode_info *vbios_mode_info;
@@ -937,7 +936,7 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
static int ast_crtc_init(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
struct ast_crtc *crtc;
int ret;
@@ -966,7 +965,7 @@ static int ast_crtc_init(struct drm_device *dev)
static int ast_encoder_init(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
struct drm_encoder *encoder = &ast->encoder;
int ret;
@@ -986,7 +985,7 @@ static int ast_encoder_init(struct drm_device *dev)
static int ast_get_modes(struct drm_connector *connector)
{
struct ast_connector *ast_connector = to_ast_connector(connector);
- struct ast_private *ast = connector->dev->dev_private;
+ struct ast_private *ast = to_ast_private(connector->dev);
struct edid *edid;
int ret;
bool flags = false;
@@ -1017,7 +1016,7 @@ static int ast_get_modes(struct drm_connector *connector)
static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
{
- struct ast_private *ast = connector->dev->dev_private;
+ struct ast_private *ast = to_ast_private(connector->dev);
int flags = MODE_NOMODE;
uint32_t jtemp;
@@ -1128,7 +1127,7 @@ static int ast_connector_init(struct drm_device *dev)
/* allocate cursor cache and pin at start of VRAM */
static int ast_cursor_init(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
size_t size, i;
struct drm_gem_vram_object *gbo;
int ret;
@@ -1166,7 +1165,7 @@ static int ast_cursor_init(struct drm_device *dev)
static void ast_cursor_fini(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
size_t i;
struct drm_gem_vram_object *gbo;
@@ -1179,7 +1178,7 @@ static void ast_cursor_fini(struct drm_device *dev)
int ast_mode_init(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
int ret;
memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
@@ -1223,7 +1222,7 @@ void ast_mode_fini(struct drm_device *dev)
static int get_clock(void *i2c_priv)
{
struct ast_i2c_chan *i2c = i2c_priv;
- struct ast_private *ast = i2c->dev->dev_private;
+ struct ast_private *ast = to_ast_private(i2c->dev);
uint32_t val, val2, count, pass;
count = 0;
@@ -1245,7 +1244,7 @@ static int get_clock(void *i2c_priv)
static int get_data(void *i2c_priv)
{
struct ast_i2c_chan *i2c = i2c_priv;
- struct ast_private *ast = i2c->dev->dev_private;
+ struct ast_private *ast = to_ast_private(i2c->dev);
uint32_t val, val2, count, pass;
count = 0;
@@ -1267,7 +1266,7 @@ static int get_data(void *i2c_priv)
static void set_clock(void *i2c_priv, int clock)
{
struct ast_i2c_chan *i2c = i2c_priv;
- struct ast_private *ast = i2c->dev->dev_private;
+ struct ast_private *ast = to_ast_private(i2c->dev);
int i;
u8 ujcrb7, jtemp;
@@ -1283,7 +1282,7 @@ static void set_clock(void *i2c_priv, int clock)
static void set_data(void *i2c_priv, int data)
{
struct ast_i2c_chan *i2c = i2c_priv;
- struct ast_private *ast = i2c->dev->dev_private;
+ struct ast_private *ast = to_ast_private(i2c->dev);
int i;
u8 ujcrb7, jtemp;
@@ -1431,7 +1430,7 @@ static int ast_cursor_move(struct drm_crtc *crtc,
int x, int y)
{
struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
- struct ast_private *ast = crtc->dev->dev_private;
+ struct ast_private *ast = to_ast_private(crtc->dev);
struct drm_gem_vram_object *gbo;
int x_offset, y_offset;
u8 *dst, *sig;
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index af0c8ebb009a1..226e1290227ad 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -39,7 +39,7 @@ static void ast_post_chip_2500(struct drm_device *dev);
void ast_enable_vga(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
ast_io_write8(ast, AST_IO_VGA_ENABLE_PORT, 0x01);
ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
@@ -47,7 +47,7 @@ void ast_enable_vga(struct drm_device *dev)
void ast_enable_mmio(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
}
@@ -55,7 +55,7 @@ void ast_enable_mmio(struct drm_device *dev)
bool ast_is_vga_enabled(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u8 ch;
ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
@@ -70,7 +70,7 @@ static const u8 extreginfo_ast2300[] = { 0x0f, 0x04, 0x1f, 0xff };
static void
ast_set_def_ext_reg(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u8 i, index, reg;
const u8 *ext_reg_info;
@@ -272,7 +272,7 @@ static void cbrdlli_ast2150(struct ast_private *ast, int busw)
static void ast_init_dram_reg(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u8 j;
u32 data, temp, i;
const struct ast_dramstruct *dram_reg_info;
@@ -366,7 +366,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
void ast_post_gpu(struct drm_device *dev)
{
u32 reg;
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
pci_read_config_dword(ast->dev->pdev, 0x04, ®);
reg |= 0x3;
@@ -1596,7 +1596,7 @@ static void ddr2_init(struct ast_private *ast, struct ast2300_dram_param *param)
static void ast_post_chip_2300(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
struct ast2300_dram_param param;
u32 temp;
u8 reg;
@@ -2028,7 +2028,7 @@ static bool ast_dram_init_2500(struct ast_private *ast)
void ast_post_chip_2500(struct drm_device *dev)
{
- struct ast_private *ast = dev->dev_private;
+ struct ast_private *ast = to_ast_private(dev);
u32 temp;
u8 reg;
--
2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] drm/ast: Use per-device logging macros
2020-06-11 8:28 [PATCH 0/3] ast cleanups Thomas Zimmermann
2020-06-11 8:28 ` [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180 Thomas Zimmermann
2020-06-11 8:28 ` [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private() Thomas Zimmermann
@ 2020-06-11 8:28 ` Thomas Zimmermann
2020-06-11 17:33 ` Daniel Vetter
2020-06-11 19:24 ` Sam Ravnborg
2 siblings, 2 replies; 17+ messages in thread
From: Thomas Zimmermann @ 2020-06-11 8:28 UTC (permalink / raw)
To: airlied, daniel, sam, emil.velikov, kraxel, noralf, chen
Cc: Thomas Zimmermann, dri-devel
Converts the ast driver to drm_info() and drm_err(). No functional
changes are made.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/ast/ast_main.c | 34 +++++++++++++++++-----------------
drivers/gpu/drm/ast/ast_mode.c | 8 ++++----
drivers/gpu/drm/ast/ast_post.c | 2 +-
drivers/gpu/drm/ast/ast_ttm.c | 2 +-
4 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index a2ef3d9077671..9063fdc9e8852 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -79,7 +79,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
scu_rev)) {
/* We do, disable P2A access */
ast->config_mode = ast_use_dt;
- DRM_INFO("Using device-tree for configuration\n");
+ drm_info(dev, "Using device-tree for configuration\n");
return;
}
@@ -101,7 +101,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
/* P2A works, grab silicon revision */
ast->config_mode = ast_use_p2a;
- DRM_INFO("Using P2A bridge for configuration\n");
+ drm_info(dev, "Using P2A bridge for configuration\n");
/* Read SCU7c (silicon revision register) */
ast_write32(ast, 0xf004, 0x1e6e0000);
@@ -112,7 +112,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
}
/* We have a P2A bridge but it's disabled */
- DRM_INFO("P2A bridge disabled, using default configuration\n");
+ drm_info(dev, "P2A bridge disabled, using default configuration\n");
}
static int ast_detect_chip(struct drm_device *dev, bool *need_post)
@@ -128,7 +128,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
*/
if (!ast_is_vga_enabled(dev)) {
ast_enable_vga(dev);
- DRM_INFO("VGA not enabled on entry, requesting chip POST\n");
+ drm_info(dev, "VGA not enabled on entry, requesting chip POST\n");
*need_post = true;
} else
*need_post = false;
@@ -144,36 +144,36 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
/* Identify chipset */
if (dev->pdev->revision >= 0x40) {
ast->chip = AST2500;
- DRM_INFO("AST 2500 detected\n");
+ drm_info(dev, "AST 2500 detected\n");
} else if (dev->pdev->revision >= 0x30) {
ast->chip = AST2400;
- DRM_INFO("AST 2400 detected\n");
+ drm_info(dev, "AST 2400 detected\n");
} else if (dev->pdev->revision >= 0x20) {
ast->chip = AST2300;
- DRM_INFO("AST 2300 detected\n");
+ drm_info(dev, "AST 2300 detected\n");
} else if (dev->pdev->revision >= 0x10) {
switch (scu_rev & 0x0300) {
case 0x0200:
ast->chip = AST1100;
- DRM_INFO("AST 1100 detected\n");
+ drm_info(dev, "AST 1100 detected\n");
break;
case 0x0100:
ast->chip = AST2200;
- DRM_INFO("AST 2200 detected\n");
+ drm_info(dev, "AST 2200 detected\n");
break;
case 0x0000:
ast->chip = AST2150;
- DRM_INFO("AST 2150 detected\n");
+ drm_info(dev, "AST 2150 detected\n");
break;
default:
ast->chip = AST2100;
- DRM_INFO("AST 2100 detected\n");
+ drm_info(dev, "AST 2100 detected\n");
break;
}
ast->vga2_clone = false;
} else {
ast->chip = AST2000;
- DRM_INFO("AST 2000 detected\n");
+ drm_info(dev, "AST 2000 detected\n");
}
/* Check if we support wide screen */
@@ -248,13 +248,13 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
/* Print stuff for diagnostic purposes */
switch(ast->tx_chip_type) {
case AST_TX_SIL164:
- DRM_INFO("Using Sil164 TMDS transmitter\n");
+ drm_info(dev, "Using Sil164 TMDS transmitter\n");
break;
case AST_TX_DP501:
- DRM_INFO("Using DP501 DisplayPort transmitter\n");
+ drm_info(dev, "Using DP501 DisplayPort transmitter\n");
break;
default:
- DRM_INFO("Analog VGA only\n");
+ drm_info(dev, "Analog VGA only\n");
}
return 0;
}
@@ -443,7 +443,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
* and higher).
*/
if (!(pci_resource_flags(dev->pdev, 2) & IORESOURCE_IO)) {
- DRM_INFO("platform has no IO space, trying MMIO\n");
+ drm_info(dev, "platform has no IO space, trying MMIO\n");
ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
}
@@ -465,7 +465,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
if (ret)
goto out_free;
ast->vram_size = ast_get_vram_info(dev);
- DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
+ drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
ast->mclk, ast->dram_type,
ast->dram_bus_width, ast->vram_size);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 10211751182da..ff789f2db9fc8 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1104,7 +1104,7 @@ static int ast_connector_init(struct drm_device *dev)
connector = &ast_connector->base;
ast_connector->i2c = ast_i2c_create(dev);
if (!ast_connector->i2c)
- DRM_ERROR("failed to add ddc bus for connector\n");
+ drm_err(dev, "failed to add ddc bus for connector\n");
drm_connector_init_with_ddc(dev, connector,
&ast_connector_funcs,
@@ -1188,7 +1188,7 @@ int ast_mode_init(struct drm_device *dev)
ARRAY_SIZE(ast_primary_plane_formats),
NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
if (ret) {
- DRM_ERROR("ast: drm_universal_plane_init() failed: %d\n", ret);
+ drm_err(dev, "ast: drm_universal_plane_init() failed: %d\n", ret);
return ret;
}
drm_plane_helper_add(&ast->primary_plane,
@@ -1200,7 +1200,7 @@ int ast_mode_init(struct drm_device *dev)
ARRAY_SIZE(ast_cursor_plane_formats),
NULL, DRM_PLANE_TYPE_CURSOR, NULL);
if (ret) {
- DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
+ drm_err(dev, "drm_universal_plane_failed(): %d\n", ret);
return ret;
}
drm_plane_helper_add(&ast->cursor_plane,
@@ -1322,7 +1322,7 @@ static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
i2c->bit.getscl = get_clock;
ret = i2c_bit_add_bus(&i2c->adapter);
if (ret) {
- DRM_ERROR("Failed to register bit i2c\n");
+ drm_err(dev, "Failed to register bit i2c\n");
goto out_free;
}
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 226e1290227ad..c043fe7175530 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -2067,7 +2067,7 @@ void ast_post_chip_2500(struct drm_device *dev)
}
if (!ast_dram_init_2500(ast))
- DRM_ERROR("DRAM init failed !\n");
+ drm_err(dev, "DRAM init failed !\n");
temp = ast_mindwm(ast, 0x1e6e2040);
ast_moutdwm(ast, 0x1e6e2040, temp | 0x40);
diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index fad34106083a8..9c3788a4c1c54 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -44,7 +44,7 @@ int ast_mm_init(struct ast_private *ast)
ast->vram_size);
if (IS_ERR(vmm)) {
ret = PTR_ERR(vmm);
- DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
+ drm_err(dev, "Error initializing VRAM MM; %d\n", ret);
return ret;
}
--
2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180
2020-06-11 8:28 ` [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180 Thomas Zimmermann
@ 2020-06-11 17:24 ` Daniel Vetter
2020-06-15 23:21 ` Emil Velikov
1 sibling, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2020-06-11 17:24 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: chen, dri-devel, kraxel, airlied, sam, emil.velikov
On Thu, Jun 11, 2020 at 10:28:07AM +0200, Thomas Zimmermann wrote:
> The ast driver contains code paths for AST 1180 chips. The chip is not
> supported and the rsp code has never been tested. Simplify the driver by
> removing the AST 1180 code.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/ast/ast_drv.c | 1 -
> drivers/gpu/drm/ast/ast_drv.h | 2 -
> drivers/gpu/drm/ast/ast_main.c | 89 +++++++++++++++-------------------
> drivers/gpu/drm/ast/ast_mode.c | 10 +---
> drivers/gpu/drm/ast/ast_post.c | 10 ++--
> 5 files changed, 43 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index b7ba22dddcad9..83509106f3ba9 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -59,7 +59,6 @@ static struct drm_driver driver;
> static const struct pci_device_id pciidlist[] = {
> AST_VGA_DEVICE(PCI_CHIP_AST2000, NULL),
> AST_VGA_DEVICE(PCI_CHIP_AST2100, NULL),
> - /* AST_VGA_DEVICE(PCI_CHIP_AST1180, NULL), - don't bind to 1180 for now */
> {0, 0, 0},
> };
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 656d591b154b3..09f2659e29118 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -52,7 +52,6 @@
>
> #define PCI_CHIP_AST2000 0x2000
> #define PCI_CHIP_AST2100 0x2010
> -#define PCI_CHIP_AST1180 0x1180
>
>
> enum ast_chip {
> @@ -64,7 +63,6 @@ enum ast_chip {
> AST2300,
> AST2400,
> AST2500,
> - AST1180,
> };
>
> enum ast_tx_chip {
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index e5398e3dabe70..f48a9f62368c0 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -142,50 +142,42 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> ast_detect_config_mode(dev, &scu_rev);
>
> /* Identify chipset */
> - if (dev->pdev->device == PCI_CHIP_AST1180) {
> - ast->chip = AST1100;
> - DRM_INFO("AST 1180 detected\n");
> - } else {
> - if (dev->pdev->revision >= 0x40) {
> - ast->chip = AST2500;
> - DRM_INFO("AST 2500 detected\n");
> - } else if (dev->pdev->revision >= 0x30) {
> - ast->chip = AST2400;
> - DRM_INFO("AST 2400 detected\n");
> - } else if (dev->pdev->revision >= 0x20) {
> - ast->chip = AST2300;
> - DRM_INFO("AST 2300 detected\n");
> - } else if (dev->pdev->revision >= 0x10) {
> - switch (scu_rev & 0x0300) {
> - case 0x0200:
> - ast->chip = AST1100;
> - DRM_INFO("AST 1100 detected\n");
> - break;
> - case 0x0100:
> - ast->chip = AST2200;
> - DRM_INFO("AST 2200 detected\n");
> - break;
> - case 0x0000:
> - ast->chip = AST2150;
> - DRM_INFO("AST 2150 detected\n");
> - break;
> - default:
> - ast->chip = AST2100;
> - DRM_INFO("AST 2100 detected\n");
> - break;
> - }
> - ast->vga2_clone = false;
> - } else {
> - ast->chip = AST2000;
> - DRM_INFO("AST 2000 detected\n");
> + if (dev->pdev->revision >= 0x40) {
> + ast->chip = AST2500;
> + DRM_INFO("AST 2500 detected\n");
> + } else if (dev->pdev->revision >= 0x30) {
> + ast->chip = AST2400;
> + DRM_INFO("AST 2400 detected\n");
> + } else if (dev->pdev->revision >= 0x20) {
> + ast->chip = AST2300;
> + DRM_INFO("AST 2300 detected\n");
> + } else if (dev->pdev->revision >= 0x10) {
> + switch (scu_rev & 0x0300) {
> + case 0x0200:
> + ast->chip = AST1100;
> + DRM_INFO("AST 1100 detected\n");
> + break;
> + case 0x0100:
> + ast->chip = AST2200;
> + DRM_INFO("AST 2200 detected\n");
> + break;
> + case 0x0000:
> + ast->chip = AST2150;
> + DRM_INFO("AST 2150 detected\n");
> + break;
> + default:
> + ast->chip = AST2100;
> + DRM_INFO("AST 2100 detected\n");
> + break;
> }
> + ast->vga2_clone = false;
> + } else {
> + ast->chip = AST2000;
> + DRM_INFO("AST 2000 detected\n");
> }
>
> /* Check if we support wide screen */
> switch (ast->chip) {
> - case AST1180:
> - ast->support_wide_screen = true;
> - break;
> case AST2000:
> ast->support_wide_screen = false;
> break;
> @@ -469,15 +461,13 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
> if (need_post)
> ast_post_gpu(dev);
>
> - if (ast->chip != AST1180) {
> - ret = ast_get_dram_info(dev);
> - if (ret)
> - goto out_free;
> - ast->vram_size = ast_get_vram_info(dev);
> - DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
> - ast->mclk, ast->dram_type,
> - ast->dram_bus_width, ast->vram_size);
> - }
> + ret = ast_get_dram_info(dev);
> + if (ret)
> + goto out_free;
> + ast->vram_size = ast_get_vram_info(dev);
> + DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
> + ast->mclk, ast->dram_type,
> + ast->dram_bus_width, ast->vram_size);
>
> ret = ast_mm_init(ast);
> if (ret)
> @@ -496,8 +486,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
> ast->chip == AST2200 ||
> ast->chip == AST2300 ||
> ast->chip == AST2400 ||
> - ast->chip == AST2500 ||
> - ast->chip == AST1180) {
> + ast->chip == AST2500) {
> dev->mode_config.max_width = 1920;
> dev->mode_config.max_height = 2048;
> } else {
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 7d39b858c9f1f..be0e2250708fa 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -768,9 +768,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
> {
> struct ast_private *ast = crtc->dev->dev_private;
>
> - if (ast->chip == AST1180)
> - return;
> -
> /* TODO: Maybe control display signal generation with
> * Sync Enable (bit CR17.7).
> */
> @@ -797,11 +794,6 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
> const struct drm_format_info *format;
> bool succ;
>
> - if (ast->chip == AST1180) {
> - DRM_ERROR("AST 1180 modesetting not supported\n");
> - return -EINVAL;
> - }
> -
> if (!state->enable)
> return 0; /* no mode checks if CRTC is being disabled */
>
> @@ -1043,7 +1035,7 @@ static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
>
> if ((ast->chip == AST2100) || (ast->chip == AST2200) ||
> (ast->chip == AST2300) || (ast->chip == AST2400) ||
> - (ast->chip == AST2500) || (ast->chip == AST1180)) {
> + (ast->chip == AST2500)) {
> if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080))
> return MODE_OK;
>
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 2d1b186197432..af0c8ebb009a1 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -58,13 +58,9 @@ bool ast_is_vga_enabled(struct drm_device *dev)
> struct ast_private *ast = dev->dev_private;
> u8 ch;
>
> - if (ast->chip == AST1180) {
> - /* TODO 1180 */
> - } else {
> - ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
> - return !!(ch & 0x01);
> - }
> - return false;
> + ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
> +
> + return !!(ch & 0x01);
> }
>
> static const u8 extreginfo[] = { 0x0f, 0x04, 0x1c, 0xff };
> --
> 2.26.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private()
2020-06-11 8:28 ` [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private() Thomas Zimmermann
@ 2020-06-11 17:32 ` Daniel Vetter
2020-06-12 6:31 ` Thomas Zimmermann
2020-06-11 19:23 ` Sam Ravnborg
1 sibling, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2020-06-11 17:32 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: chen, dri-devel, kraxel, airlied, sam, emil.velikov
On Thu, Jun 11, 2020 at 10:28:08AM +0200, Thomas Zimmermann wrote:
> All upcasting from struct drm_device to struct ast_private is now
> performed via to_ast_private(). Using struct drm_device.dev_private is
> deprecated. The ast variable in ast_crtc_helperatomic_check() is unused,
> so removed it.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Aside, the check in ast_pm_freeze is bogus, you can't resume/freeze before
the driver has completed loading.
I suspect this is a remnant from the old days of dri1 where freeze/resume
before the driver finished loading was very much possible with the shadow
attach stuff. So probably just copypasta stuff.
In general when you spot that in a modern kms driver, then just delete it.
that = checking whether the drm_device or dev_private is set. Definitely
not a pattern we should propagate.
Cheers, Daniel
> ---
> drivers/gpu/drm/ast/ast_dp501.c | 24 +++++++++----------
> drivers/gpu/drm/ast/ast_drv.h | 5 ++++
> drivers/gpu/drm/ast/ast_main.c | 10 ++++----
> drivers/gpu/drm/ast/ast_mode.c | 41 ++++++++++++++++-----------------
> drivers/gpu/drm/ast/ast_post.c | 16 ++++++-------
> 5 files changed, 50 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
> index 98cd69269263f..4b85a504825a2 100644
> --- a/drivers/gpu/drm/ast/ast_dp501.c
> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> @@ -10,7 +10,7 @@ MODULE_FIRMWARE("ast_dp501_fw.bin");
>
> static int ast_load_dp501_microcode(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> return request_firmware(&ast->dp501_fw, "ast_dp501_fw.bin", dev->dev);
> }
> @@ -93,7 +93,7 @@ static bool wait_fw_ready(struct ast_private *ast)
>
> static bool ast_write_cmd(struct drm_device *dev, u8 data)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> int retry = 0;
> if (wait_nack(ast)) {
> send_nack(ast);
> @@ -115,7 +115,7 @@ static bool ast_write_cmd(struct drm_device *dev, u8 data)
>
> static bool ast_write_data(struct drm_device *dev, u8 data)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> if (wait_nack(ast)) {
> send_nack(ast);
> @@ -133,7 +133,7 @@ static bool ast_write_data(struct drm_device *dev, u8 data)
> #if 0
> static bool ast_read_data(struct drm_device *dev, u8 *data)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 tmp;
>
> *data = 0;
> @@ -172,7 +172,7 @@ static u32 get_fw_base(struct ast_private *ast)
>
> bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 i, data;
> u32 boot_address;
>
> @@ -188,7 +188,7 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>
> static bool ast_launch_m68k(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 i, data, len = 0;
> u32 boot_address;
> u8 *fw_addr = NULL;
> @@ -255,7 +255,7 @@ static bool ast_launch_m68k(struct drm_device *dev)
>
> u8 ast_get_dp501_max_clk(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 boot_address, offset, data;
> u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
>
> @@ -283,7 +283,7 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>
> bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 i, boot_address, offset, data;
>
> boot_address = get_fw_base(ast);
> @@ -312,7 +312,7 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>
> static bool ast_init_dvo(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 jreg;
> u32 data;
> ast_write32(ast, 0xf004, 0x1e6e0000);
> @@ -385,7 +385,7 @@ static bool ast_init_dvo(struct drm_device *dev)
>
> static void ast_init_analog(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 data;
>
> /*
> @@ -412,7 +412,7 @@ static void ast_init_analog(struct drm_device *dev)
>
> void ast_init_3rdtx(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 jreg;
>
> if (ast->chip == AST2300 || ast->chip == AST2400) {
> @@ -438,7 +438,7 @@ void ast_init_3rdtx(struct drm_device *dev)
>
> void ast_release_firmware(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> release_firmware(ast->dp501_fw);
> ast->dp501_fw = NULL;
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 09f2659e29118..c44c1376c6977 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -136,6 +136,11 @@ struct ast_private {
> const struct firmware *dp501_fw; /* dp501 fw */
> };
>
> +static inline struct ast_private *to_ast_private(struct drm_device *dev)
> +{
> + return dev->dev_private;
> +}
> +
> int ast_driver_load(struct drm_device *dev, unsigned long flags);
> void ast_driver_unload(struct drm_device *dev);
>
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index f48a9f62368c0..a2ef3d9077671 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -67,7 +67,7 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
> static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> {
> struct device_node *np = dev->pdev->dev.of_node;
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> uint32_t data, jregd0, jregd1;
>
> /* Defaults */
> @@ -117,7 +117,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>
> static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> uint32_t jreg, scu_rev;
>
> /*
> @@ -262,7 +262,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> static int ast_get_dram_info(struct drm_device *dev)
> {
> struct device_node *np = dev->pdev->dev.of_node;
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
> uint32_t denum, num, div, ref_pll, dsel;
>
> @@ -388,7 +388,7 @@ static const struct drm_mode_config_funcs ast_mode_funcs = {
>
> static u32 ast_get_vram_info(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 jreg;
> u32 vram_size;
> ast_open_key(ast);
> @@ -509,7 +509,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>
> void ast_driver_unload(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> /* enable standard VGA decode */
> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index be0e2250708fa..10211751182da 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -565,7 +565,7 @@ static void
> ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> - struct ast_private *ast = plane->dev->dev_private;
> + struct ast_private *ast = to_ast_private(plane->dev);
> struct drm_plane_state *state = plane->state;
> struct drm_gem_vram_object *gbo;
> s64 gpu_addr;
> @@ -585,7 +585,7 @@ static void
> ast_primary_plane_helper_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> - struct ast_private *ast = plane->dev->dev_private;
> + struct ast_private *ast = to_ast_private(plane->dev);
>
> ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x20);
> }
> @@ -633,7 +633,7 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
> WARN_ON_ONCE(fb->height > AST_MAX_HWC_HEIGHT))
> return -EINVAL; /* BUG: didn't test in atomic_check() */
>
> - ast = crtc->dev->dev_private;
> + ast = to_ast_private(crtc->dev);
>
> gbo = drm_gem_vram_of_gem(fb->obj[0]);
> src = drm_gem_vram_vmap(gbo);
> @@ -705,7 +705,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
> struct drm_plane_state *state = plane->state;
> struct drm_crtc *crtc = state->crtc;
> struct drm_framebuffer *fb = state->fb;
> - struct ast_private *ast = plane->dev->dev_private;
> + struct ast_private *ast = to_ast_private(plane->dev);
> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> struct drm_gem_vram_object *gbo;
> s64 off;
> @@ -738,7 +738,7 @@ static void
> ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> - struct ast_private *ast = plane->dev->dev_private;
> + struct ast_private *ast = to_ast_private(plane->dev);
>
> ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
> }
> @@ -766,7 +766,7 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs = {
>
> static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
> {
> - struct ast_private *ast = crtc->dev->dev_private;
> + struct ast_private *ast = to_ast_private(crtc->dev);
>
> /* TODO: Maybe control display signal generation with
> * Sync Enable (bit CR17.7).
> @@ -789,7 +789,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
> static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
> struct drm_crtc_state *state)
> {
> - struct ast_private *ast = crtc->dev->dev_private;
> struct ast_crtc_state *ast_state;
> const struct drm_format_info *format;
> bool succ;
> @@ -815,7 +814,7 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
> static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc,
> struct drm_crtc_state *old_crtc_state)
> {
> - struct ast_private *ast = crtc->dev->dev_private;
> + struct ast_private *ast = to_ast_private(crtc->dev);
>
> ast_open_key(ast);
> }
> @@ -824,7 +823,7 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
> struct drm_crtc_state *old_crtc_state)
> {
> struct drm_device *dev = crtc->dev;
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> struct ast_crtc_state *ast_state;
> const struct drm_format_info *format;
> struct ast_vbios_mode_info *vbios_mode_info;
> @@ -937,7 +936,7 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
>
> static int ast_crtc_init(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> struct ast_crtc *crtc;
> int ret;
>
> @@ -966,7 +965,7 @@ static int ast_crtc_init(struct drm_device *dev)
>
> static int ast_encoder_init(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> struct drm_encoder *encoder = &ast->encoder;
> int ret;
>
> @@ -986,7 +985,7 @@ static int ast_encoder_init(struct drm_device *dev)
> static int ast_get_modes(struct drm_connector *connector)
> {
> struct ast_connector *ast_connector = to_ast_connector(connector);
> - struct ast_private *ast = connector->dev->dev_private;
> + struct ast_private *ast = to_ast_private(connector->dev);
> struct edid *edid;
> int ret;
> bool flags = false;
> @@ -1017,7 +1016,7 @@ static int ast_get_modes(struct drm_connector *connector)
> static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> {
> - struct ast_private *ast = connector->dev->dev_private;
> + struct ast_private *ast = to_ast_private(connector->dev);
> int flags = MODE_NOMODE;
> uint32_t jtemp;
>
> @@ -1128,7 +1127,7 @@ static int ast_connector_init(struct drm_device *dev)
> /* allocate cursor cache and pin at start of VRAM */
> static int ast_cursor_init(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> size_t size, i;
> struct drm_gem_vram_object *gbo;
> int ret;
> @@ -1166,7 +1165,7 @@ static int ast_cursor_init(struct drm_device *dev)
>
> static void ast_cursor_fini(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> size_t i;
> struct drm_gem_vram_object *gbo;
>
> @@ -1179,7 +1178,7 @@ static void ast_cursor_fini(struct drm_device *dev)
>
> int ast_mode_init(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> int ret;
>
> memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
> @@ -1223,7 +1222,7 @@ void ast_mode_fini(struct drm_device *dev)
> static int get_clock(void *i2c_priv)
> {
> struct ast_i2c_chan *i2c = i2c_priv;
> - struct ast_private *ast = i2c->dev->dev_private;
> + struct ast_private *ast = to_ast_private(i2c->dev);
> uint32_t val, val2, count, pass;
>
> count = 0;
> @@ -1245,7 +1244,7 @@ static int get_clock(void *i2c_priv)
> static int get_data(void *i2c_priv)
> {
> struct ast_i2c_chan *i2c = i2c_priv;
> - struct ast_private *ast = i2c->dev->dev_private;
> + struct ast_private *ast = to_ast_private(i2c->dev);
> uint32_t val, val2, count, pass;
>
> count = 0;
> @@ -1267,7 +1266,7 @@ static int get_data(void *i2c_priv)
> static void set_clock(void *i2c_priv, int clock)
> {
> struct ast_i2c_chan *i2c = i2c_priv;
> - struct ast_private *ast = i2c->dev->dev_private;
> + struct ast_private *ast = to_ast_private(i2c->dev);
> int i;
> u8 ujcrb7, jtemp;
>
> @@ -1283,7 +1282,7 @@ static void set_clock(void *i2c_priv, int clock)
> static void set_data(void *i2c_priv, int data)
> {
> struct ast_i2c_chan *i2c = i2c_priv;
> - struct ast_private *ast = i2c->dev->dev_private;
> + struct ast_private *ast = to_ast_private(i2c->dev);
> int i;
> u8 ujcrb7, jtemp;
>
> @@ -1431,7 +1430,7 @@ static int ast_cursor_move(struct drm_crtc *crtc,
> int x, int y)
> {
> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> - struct ast_private *ast = crtc->dev->dev_private;
> + struct ast_private *ast = to_ast_private(crtc->dev);
> struct drm_gem_vram_object *gbo;
> int x_offset, y_offset;
> u8 *dst, *sig;
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index af0c8ebb009a1..226e1290227ad 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -39,7 +39,7 @@ static void ast_post_chip_2500(struct drm_device *dev);
>
> void ast_enable_vga(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> ast_io_write8(ast, AST_IO_VGA_ENABLE_PORT, 0x01);
> ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
> @@ -47,7 +47,7 @@ void ast_enable_vga(struct drm_device *dev)
>
> void ast_enable_mmio(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
> }
> @@ -55,7 +55,7 @@ void ast_enable_mmio(struct drm_device *dev)
>
> bool ast_is_vga_enabled(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 ch;
>
> ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
> @@ -70,7 +70,7 @@ static const u8 extreginfo_ast2300[] = { 0x0f, 0x04, 0x1f, 0xff };
> static void
> ast_set_def_ext_reg(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 i, index, reg;
> const u8 *ext_reg_info;
>
> @@ -272,7 +272,7 @@ static void cbrdlli_ast2150(struct ast_private *ast, int busw)
>
> static void ast_init_dram_reg(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 j;
> u32 data, temp, i;
> const struct ast_dramstruct *dram_reg_info;
> @@ -366,7 +366,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
> void ast_post_gpu(struct drm_device *dev)
> {
> u32 reg;
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> pci_read_config_dword(ast->dev->pdev, 0x04, ®);
> reg |= 0x3;
> @@ -1596,7 +1596,7 @@ static void ddr2_init(struct ast_private *ast, struct ast2300_dram_param *param)
>
> static void ast_post_chip_2300(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> struct ast2300_dram_param param;
> u32 temp;
> u8 reg;
> @@ -2028,7 +2028,7 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>
> void ast_post_chip_2500(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 temp;
> u8 reg;
>
> --
> 2.26.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] drm/ast: Use per-device logging macros
2020-06-11 8:28 ` [PATCH 3/3] drm/ast: Use per-device logging macros Thomas Zimmermann
@ 2020-06-11 17:33 ` Daniel Vetter
2020-06-11 19:24 ` Sam Ravnborg
1 sibling, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2020-06-11 17:33 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: chen, dri-devel, kraxel, airlied, sam, emil.velikov
On Thu, Jun 11, 2020 at 10:28:09AM +0200, Thomas Zimmermann wrote:
> Converts the ast driver to drm_info() and drm_err(). No functional
> changes are made.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
I didn't check whether it compiles, but looks all good to me.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/ast/ast_main.c | 34 +++++++++++++++++-----------------
> drivers/gpu/drm/ast/ast_mode.c | 8 ++++----
> drivers/gpu/drm/ast/ast_post.c | 2 +-
> drivers/gpu/drm/ast/ast_ttm.c | 2 +-
> 4 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index a2ef3d9077671..9063fdc9e8852 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -79,7 +79,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> scu_rev)) {
> /* We do, disable P2A access */
> ast->config_mode = ast_use_dt;
> - DRM_INFO("Using device-tree for configuration\n");
> + drm_info(dev, "Using device-tree for configuration\n");
> return;
> }
>
> @@ -101,7 +101,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> /* P2A works, grab silicon revision */
> ast->config_mode = ast_use_p2a;
>
> - DRM_INFO("Using P2A bridge for configuration\n");
> + drm_info(dev, "Using P2A bridge for configuration\n");
>
> /* Read SCU7c (silicon revision register) */
> ast_write32(ast, 0xf004, 0x1e6e0000);
> @@ -112,7 +112,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> }
>
> /* We have a P2A bridge but it's disabled */
> - DRM_INFO("P2A bridge disabled, using default configuration\n");
> + drm_info(dev, "P2A bridge disabled, using default configuration\n");
> }
>
> static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> @@ -128,7 +128,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> */
> if (!ast_is_vga_enabled(dev)) {
> ast_enable_vga(dev);
> - DRM_INFO("VGA not enabled on entry, requesting chip POST\n");
> + drm_info(dev, "VGA not enabled on entry, requesting chip POST\n");
> *need_post = true;
> } else
> *need_post = false;
> @@ -144,36 +144,36 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> /* Identify chipset */
> if (dev->pdev->revision >= 0x40) {
> ast->chip = AST2500;
> - DRM_INFO("AST 2500 detected\n");
> + drm_info(dev, "AST 2500 detected\n");
> } else if (dev->pdev->revision >= 0x30) {
> ast->chip = AST2400;
> - DRM_INFO("AST 2400 detected\n");
> + drm_info(dev, "AST 2400 detected\n");
> } else if (dev->pdev->revision >= 0x20) {
> ast->chip = AST2300;
> - DRM_INFO("AST 2300 detected\n");
> + drm_info(dev, "AST 2300 detected\n");
> } else if (dev->pdev->revision >= 0x10) {
> switch (scu_rev & 0x0300) {
> case 0x0200:
> ast->chip = AST1100;
> - DRM_INFO("AST 1100 detected\n");
> + drm_info(dev, "AST 1100 detected\n");
> break;
> case 0x0100:
> ast->chip = AST2200;
> - DRM_INFO("AST 2200 detected\n");
> + drm_info(dev, "AST 2200 detected\n");
> break;
> case 0x0000:
> ast->chip = AST2150;
> - DRM_INFO("AST 2150 detected\n");
> + drm_info(dev, "AST 2150 detected\n");
> break;
> default:
> ast->chip = AST2100;
> - DRM_INFO("AST 2100 detected\n");
> + drm_info(dev, "AST 2100 detected\n");
> break;
> }
> ast->vga2_clone = false;
> } else {
> ast->chip = AST2000;
> - DRM_INFO("AST 2000 detected\n");
> + drm_info(dev, "AST 2000 detected\n");
> }
>
> /* Check if we support wide screen */
> @@ -248,13 +248,13 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> /* Print stuff for diagnostic purposes */
> switch(ast->tx_chip_type) {
> case AST_TX_SIL164:
> - DRM_INFO("Using Sil164 TMDS transmitter\n");
> + drm_info(dev, "Using Sil164 TMDS transmitter\n");
> break;
> case AST_TX_DP501:
> - DRM_INFO("Using DP501 DisplayPort transmitter\n");
> + drm_info(dev, "Using DP501 DisplayPort transmitter\n");
> break;
> default:
> - DRM_INFO("Analog VGA only\n");
> + drm_info(dev, "Analog VGA only\n");
> }
> return 0;
> }
> @@ -443,7 +443,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
> * and higher).
> */
> if (!(pci_resource_flags(dev->pdev, 2) & IORESOURCE_IO)) {
> - DRM_INFO("platform has no IO space, trying MMIO\n");
> + drm_info(dev, "platform has no IO space, trying MMIO\n");
> ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
> }
>
> @@ -465,7 +465,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
> if (ret)
> goto out_free;
> ast->vram_size = ast_get_vram_info(dev);
> - DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
> + drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
> ast->mclk, ast->dram_type,
> ast->dram_bus_width, ast->vram_size);
>
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 10211751182da..ff789f2db9fc8 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1104,7 +1104,7 @@ static int ast_connector_init(struct drm_device *dev)
> connector = &ast_connector->base;
> ast_connector->i2c = ast_i2c_create(dev);
> if (!ast_connector->i2c)
> - DRM_ERROR("failed to add ddc bus for connector\n");
> + drm_err(dev, "failed to add ddc bus for connector\n");
>
> drm_connector_init_with_ddc(dev, connector,
> &ast_connector_funcs,
> @@ -1188,7 +1188,7 @@ int ast_mode_init(struct drm_device *dev)
> ARRAY_SIZE(ast_primary_plane_formats),
> NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> - DRM_ERROR("ast: drm_universal_plane_init() failed: %d\n", ret);
> + drm_err(dev, "ast: drm_universal_plane_init() failed: %d\n", ret);
> return ret;
> }
> drm_plane_helper_add(&ast->primary_plane,
> @@ -1200,7 +1200,7 @@ int ast_mode_init(struct drm_device *dev)
> ARRAY_SIZE(ast_cursor_plane_formats),
> NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> if (ret) {
> - DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
> + drm_err(dev, "drm_universal_plane_failed(): %d\n", ret);
> return ret;
> }
> drm_plane_helper_add(&ast->cursor_plane,
> @@ -1322,7 +1322,7 @@ static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
> i2c->bit.getscl = get_clock;
> ret = i2c_bit_add_bus(&i2c->adapter);
> if (ret) {
> - DRM_ERROR("Failed to register bit i2c\n");
> + drm_err(dev, "Failed to register bit i2c\n");
> goto out_free;
> }
>
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 226e1290227ad..c043fe7175530 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2067,7 +2067,7 @@ void ast_post_chip_2500(struct drm_device *dev)
> }
>
> if (!ast_dram_init_2500(ast))
> - DRM_ERROR("DRAM init failed !\n");
> + drm_err(dev, "DRAM init failed !\n");
>
> temp = ast_mindwm(ast, 0x1e6e2040);
> ast_moutdwm(ast, 0x1e6e2040, temp | 0x40);
> diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
> index fad34106083a8..9c3788a4c1c54 100644
> --- a/drivers/gpu/drm/ast/ast_ttm.c
> +++ b/drivers/gpu/drm/ast/ast_ttm.c
> @@ -44,7 +44,7 @@ int ast_mm_init(struct ast_private *ast)
> ast->vram_size);
> if (IS_ERR(vmm)) {
> ret = PTR_ERR(vmm);
> - DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
> + drm_err(dev, "Error initializing VRAM MM; %d\n", ret);
> return ret;
> }
>
> --
> 2.26.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private()
2020-06-11 8:28 ` [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private() Thomas Zimmermann
2020-06-11 17:32 ` Daniel Vetter
@ 2020-06-11 19:23 ` Sam Ravnborg
2020-06-12 6:30 ` Thomas Zimmermann
1 sibling, 1 reply; 17+ messages in thread
From: Sam Ravnborg @ 2020-06-11 19:23 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: chen, dri-devel, kraxel, airlied, emil.velikov
Hi Thomas.
On Thu, Jun 11, 2020 at 10:28:08AM +0200, Thomas Zimmermann wrote:
> All upcasting from struct drm_device to struct ast_private is now
> performed via to_ast_private(). Using struct drm_device.dev_private is
> deprecated.
This is a simple 1:1 conversion.
But some cases - I checked ast_set_dp501_video_output() =>
ast_write_cmd(), ast_write_data()
could have been fixed by passing ast_private * rather than drm_device *.
And then no upcasting was needed.
That a more involved approach - but wanted to point it out.
Maybe for another day..
Sam
The ast variable in ast_crtc_helperatomic_check() is unused,
s/ast_crtc_helperatomic_check/ast_crtc_helper_atomic_check/
> so removed it.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
> drivers/gpu/drm/ast/ast_dp501.c | 24 +++++++++----------
> drivers/gpu/drm/ast/ast_drv.h | 5 ++++
> drivers/gpu/drm/ast/ast_main.c | 10 ++++----
> drivers/gpu/drm/ast/ast_mode.c | 41 ++++++++++++++++-----------------
> drivers/gpu/drm/ast/ast_post.c | 16 ++++++-------
> 5 files changed, 50 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
> index 98cd69269263f..4b85a504825a2 100644
> --- a/drivers/gpu/drm/ast/ast_dp501.c
> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> @@ -10,7 +10,7 @@ MODULE_FIRMWARE("ast_dp501_fw.bin");
>
> static int ast_load_dp501_microcode(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> return request_firmware(&ast->dp501_fw, "ast_dp501_fw.bin", dev->dev);
> }
> @@ -93,7 +93,7 @@ static bool wait_fw_ready(struct ast_private *ast)
>
> static bool ast_write_cmd(struct drm_device *dev, u8 data)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> int retry = 0;
> if (wait_nack(ast)) {
> send_nack(ast);
> @@ -115,7 +115,7 @@ static bool ast_write_cmd(struct drm_device *dev, u8 data)
>
> static bool ast_write_data(struct drm_device *dev, u8 data)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> if (wait_nack(ast)) {
> send_nack(ast);
> @@ -133,7 +133,7 @@ static bool ast_write_data(struct drm_device *dev, u8 data)
> #if 0
> static bool ast_read_data(struct drm_device *dev, u8 *data)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 tmp;
>
> *data = 0;
> @@ -172,7 +172,7 @@ static u32 get_fw_base(struct ast_private *ast)
>
> bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 i, data;
> u32 boot_address;
>
> @@ -188,7 +188,7 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>
> static bool ast_launch_m68k(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 i, data, len = 0;
> u32 boot_address;
> u8 *fw_addr = NULL;
> @@ -255,7 +255,7 @@ static bool ast_launch_m68k(struct drm_device *dev)
>
> u8 ast_get_dp501_max_clk(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 boot_address, offset, data;
> u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
>
> @@ -283,7 +283,7 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>
> bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 i, boot_address, offset, data;
>
> boot_address = get_fw_base(ast);
> @@ -312,7 +312,7 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>
> static bool ast_init_dvo(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 jreg;
> u32 data;
> ast_write32(ast, 0xf004, 0x1e6e0000);
> @@ -385,7 +385,7 @@ static bool ast_init_dvo(struct drm_device *dev)
>
> static void ast_init_analog(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 data;
>
> /*
> @@ -412,7 +412,7 @@ static void ast_init_analog(struct drm_device *dev)
>
> void ast_init_3rdtx(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 jreg;
>
> if (ast->chip == AST2300 || ast->chip == AST2400) {
> @@ -438,7 +438,7 @@ void ast_init_3rdtx(struct drm_device *dev)
>
> void ast_release_firmware(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> release_firmware(ast->dp501_fw);
> ast->dp501_fw = NULL;
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 09f2659e29118..c44c1376c6977 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -136,6 +136,11 @@ struct ast_private {
> const struct firmware *dp501_fw; /* dp501 fw */
> };
>
> +static inline struct ast_private *to_ast_private(struct drm_device *dev)
> +{
> + return dev->dev_private;
> +}
> +
> int ast_driver_load(struct drm_device *dev, unsigned long flags);
> void ast_driver_unload(struct drm_device *dev);
>
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index f48a9f62368c0..a2ef3d9077671 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -67,7 +67,7 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
> static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> {
> struct device_node *np = dev->pdev->dev.of_node;
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> uint32_t data, jregd0, jregd1;
>
> /* Defaults */
> @@ -117,7 +117,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>
> static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> uint32_t jreg, scu_rev;
>
> /*
> @@ -262,7 +262,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> static int ast_get_dram_info(struct drm_device *dev)
> {
> struct device_node *np = dev->pdev->dev.of_node;
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
> uint32_t denum, num, div, ref_pll, dsel;
>
> @@ -388,7 +388,7 @@ static const struct drm_mode_config_funcs ast_mode_funcs = {
>
> static u32 ast_get_vram_info(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 jreg;
> u32 vram_size;
> ast_open_key(ast);
> @@ -509,7 +509,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>
> void ast_driver_unload(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> /* enable standard VGA decode */
> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index be0e2250708fa..10211751182da 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -565,7 +565,7 @@ static void
> ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> - struct ast_private *ast = plane->dev->dev_private;
> + struct ast_private *ast = to_ast_private(plane->dev);
> struct drm_plane_state *state = plane->state;
> struct drm_gem_vram_object *gbo;
> s64 gpu_addr;
> @@ -585,7 +585,7 @@ static void
> ast_primary_plane_helper_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> - struct ast_private *ast = plane->dev->dev_private;
> + struct ast_private *ast = to_ast_private(plane->dev);
>
> ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x20);
> }
> @@ -633,7 +633,7 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
> WARN_ON_ONCE(fb->height > AST_MAX_HWC_HEIGHT))
> return -EINVAL; /* BUG: didn't test in atomic_check() */
>
> - ast = crtc->dev->dev_private;
> + ast = to_ast_private(crtc->dev);
>
> gbo = drm_gem_vram_of_gem(fb->obj[0]);
> src = drm_gem_vram_vmap(gbo);
> @@ -705,7 +705,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
> struct drm_plane_state *state = plane->state;
> struct drm_crtc *crtc = state->crtc;
> struct drm_framebuffer *fb = state->fb;
> - struct ast_private *ast = plane->dev->dev_private;
> + struct ast_private *ast = to_ast_private(plane->dev);
> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> struct drm_gem_vram_object *gbo;
> s64 off;
> @@ -738,7 +738,7 @@ static void
> ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> - struct ast_private *ast = plane->dev->dev_private;
> + struct ast_private *ast = to_ast_private(plane->dev);
>
> ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
> }
> @@ -766,7 +766,7 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs = {
>
> static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
> {
> - struct ast_private *ast = crtc->dev->dev_private;
> + struct ast_private *ast = to_ast_private(crtc->dev);
>
> /* TODO: Maybe control display signal generation with
> * Sync Enable (bit CR17.7).
> @@ -789,7 +789,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
> static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
> struct drm_crtc_state *state)
> {
> - struct ast_private *ast = crtc->dev->dev_private;
> struct ast_crtc_state *ast_state;
> const struct drm_format_info *format;
> bool succ;
> @@ -815,7 +814,7 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
> static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc,
> struct drm_crtc_state *old_crtc_state)
> {
> - struct ast_private *ast = crtc->dev->dev_private;
> + struct ast_private *ast = to_ast_private(crtc->dev);
>
> ast_open_key(ast);
> }
> @@ -824,7 +823,7 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
> struct drm_crtc_state *old_crtc_state)
> {
> struct drm_device *dev = crtc->dev;
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> struct ast_crtc_state *ast_state;
> const struct drm_format_info *format;
> struct ast_vbios_mode_info *vbios_mode_info;
> @@ -937,7 +936,7 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
>
> static int ast_crtc_init(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> struct ast_crtc *crtc;
> int ret;
>
> @@ -966,7 +965,7 @@ static int ast_crtc_init(struct drm_device *dev)
>
> static int ast_encoder_init(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> struct drm_encoder *encoder = &ast->encoder;
> int ret;
>
> @@ -986,7 +985,7 @@ static int ast_encoder_init(struct drm_device *dev)
> static int ast_get_modes(struct drm_connector *connector)
> {
> struct ast_connector *ast_connector = to_ast_connector(connector);
> - struct ast_private *ast = connector->dev->dev_private;
> + struct ast_private *ast = to_ast_private(connector->dev);
> struct edid *edid;
> int ret;
> bool flags = false;
> @@ -1017,7 +1016,7 @@ static int ast_get_modes(struct drm_connector *connector)
> static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> {
> - struct ast_private *ast = connector->dev->dev_private;
> + struct ast_private *ast = to_ast_private(connector->dev);
> int flags = MODE_NOMODE;
> uint32_t jtemp;
>
> @@ -1128,7 +1127,7 @@ static int ast_connector_init(struct drm_device *dev)
> /* allocate cursor cache and pin at start of VRAM */
> static int ast_cursor_init(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> size_t size, i;
> struct drm_gem_vram_object *gbo;
> int ret;
> @@ -1166,7 +1165,7 @@ static int ast_cursor_init(struct drm_device *dev)
>
> static void ast_cursor_fini(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> size_t i;
> struct drm_gem_vram_object *gbo;
>
> @@ -1179,7 +1178,7 @@ static void ast_cursor_fini(struct drm_device *dev)
>
> int ast_mode_init(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> int ret;
>
> memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
> @@ -1223,7 +1222,7 @@ void ast_mode_fini(struct drm_device *dev)
> static int get_clock(void *i2c_priv)
> {
> struct ast_i2c_chan *i2c = i2c_priv;
> - struct ast_private *ast = i2c->dev->dev_private;
> + struct ast_private *ast = to_ast_private(i2c->dev);
> uint32_t val, val2, count, pass;
>
> count = 0;
> @@ -1245,7 +1244,7 @@ static int get_clock(void *i2c_priv)
> static int get_data(void *i2c_priv)
> {
> struct ast_i2c_chan *i2c = i2c_priv;
> - struct ast_private *ast = i2c->dev->dev_private;
> + struct ast_private *ast = to_ast_private(i2c->dev);
> uint32_t val, val2, count, pass;
>
> count = 0;
> @@ -1267,7 +1266,7 @@ static int get_data(void *i2c_priv)
> static void set_clock(void *i2c_priv, int clock)
> {
> struct ast_i2c_chan *i2c = i2c_priv;
> - struct ast_private *ast = i2c->dev->dev_private;
> + struct ast_private *ast = to_ast_private(i2c->dev);
> int i;
> u8 ujcrb7, jtemp;
>
> @@ -1283,7 +1282,7 @@ static void set_clock(void *i2c_priv, int clock)
> static void set_data(void *i2c_priv, int data)
> {
> struct ast_i2c_chan *i2c = i2c_priv;
> - struct ast_private *ast = i2c->dev->dev_private;
> + struct ast_private *ast = to_ast_private(i2c->dev);
> int i;
> u8 ujcrb7, jtemp;
>
> @@ -1431,7 +1430,7 @@ static int ast_cursor_move(struct drm_crtc *crtc,
> int x, int y)
> {
> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> - struct ast_private *ast = crtc->dev->dev_private;
> + struct ast_private *ast = to_ast_private(crtc->dev);
> struct drm_gem_vram_object *gbo;
> int x_offset, y_offset;
> u8 *dst, *sig;
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index af0c8ebb009a1..226e1290227ad 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -39,7 +39,7 @@ static void ast_post_chip_2500(struct drm_device *dev);
>
> void ast_enable_vga(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> ast_io_write8(ast, AST_IO_VGA_ENABLE_PORT, 0x01);
> ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
> @@ -47,7 +47,7 @@ void ast_enable_vga(struct drm_device *dev)
>
> void ast_enable_mmio(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
> }
> @@ -55,7 +55,7 @@ void ast_enable_mmio(struct drm_device *dev)
>
> bool ast_is_vga_enabled(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 ch;
>
> ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
> @@ -70,7 +70,7 @@ static const u8 extreginfo_ast2300[] = { 0x0f, 0x04, 0x1f, 0xff };
> static void
> ast_set_def_ext_reg(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 i, index, reg;
> const u8 *ext_reg_info;
>
> @@ -272,7 +272,7 @@ static void cbrdlli_ast2150(struct ast_private *ast, int busw)
>
> static void ast_init_dram_reg(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u8 j;
> u32 data, temp, i;
> const struct ast_dramstruct *dram_reg_info;
> @@ -366,7 +366,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
> void ast_post_gpu(struct drm_device *dev)
> {
> u32 reg;
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
>
> pci_read_config_dword(ast->dev->pdev, 0x04, ®);
> reg |= 0x3;
> @@ -1596,7 +1596,7 @@ static void ddr2_init(struct ast_private *ast, struct ast2300_dram_param *param)
>
> static void ast_post_chip_2300(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> struct ast2300_dram_param param;
> u32 temp;
> u8 reg;
> @@ -2028,7 +2028,7 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>
> void ast_post_chip_2500(struct drm_device *dev)
> {
> - struct ast_private *ast = dev->dev_private;
> + struct ast_private *ast = to_ast_private(dev);
> u32 temp;
> u8 reg;
>
> --
> 2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] drm/ast: Use per-device logging macros
2020-06-11 8:28 ` [PATCH 3/3] drm/ast: Use per-device logging macros Thomas Zimmermann
2020-06-11 17:33 ` Daniel Vetter
@ 2020-06-11 19:24 ` Sam Ravnborg
2020-06-12 6:28 ` Thomas Zimmermann
1 sibling, 1 reply; 17+ messages in thread
From: Sam Ravnborg @ 2020-06-11 19:24 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: chen, dri-devel, kraxel, airlied, emil.velikov
Hi Thomas.
On Thu, Jun 11, 2020 at 10:28:09AM +0200, Thomas Zimmermann wrote:
> Converts the ast driver to drm_info() and drm_err(). No functional
> changes are made.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
I hope you will later follow-up with a patch that introduces drm_WARN_*.
Sam
> ---
> drivers/gpu/drm/ast/ast_main.c | 34 +++++++++++++++++-----------------
> drivers/gpu/drm/ast/ast_mode.c | 8 ++++----
> drivers/gpu/drm/ast/ast_post.c | 2 +-
> drivers/gpu/drm/ast/ast_ttm.c | 2 +-
> 4 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index a2ef3d9077671..9063fdc9e8852 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -79,7 +79,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> scu_rev)) {
> /* We do, disable P2A access */
> ast->config_mode = ast_use_dt;
> - DRM_INFO("Using device-tree for configuration\n");
> + drm_info(dev, "Using device-tree for configuration\n");
> return;
> }
>
> @@ -101,7 +101,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> /* P2A works, grab silicon revision */
> ast->config_mode = ast_use_p2a;
>
> - DRM_INFO("Using P2A bridge for configuration\n");
> + drm_info(dev, "Using P2A bridge for configuration\n");
>
> /* Read SCU7c (silicon revision register) */
> ast_write32(ast, 0xf004, 0x1e6e0000);
> @@ -112,7 +112,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> }
>
> /* We have a P2A bridge but it's disabled */
> - DRM_INFO("P2A bridge disabled, using default configuration\n");
> + drm_info(dev, "P2A bridge disabled, using default configuration\n");
> }
>
> static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> @@ -128,7 +128,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> */
> if (!ast_is_vga_enabled(dev)) {
> ast_enable_vga(dev);
> - DRM_INFO("VGA not enabled on entry, requesting chip POST\n");
> + drm_info(dev, "VGA not enabled on entry, requesting chip POST\n");
> *need_post = true;
> } else
> *need_post = false;
> @@ -144,36 +144,36 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> /* Identify chipset */
> if (dev->pdev->revision >= 0x40) {
> ast->chip = AST2500;
> - DRM_INFO("AST 2500 detected\n");
> + drm_info(dev, "AST 2500 detected\n");
> } else if (dev->pdev->revision >= 0x30) {
> ast->chip = AST2400;
> - DRM_INFO("AST 2400 detected\n");
> + drm_info(dev, "AST 2400 detected\n");
> } else if (dev->pdev->revision >= 0x20) {
> ast->chip = AST2300;
> - DRM_INFO("AST 2300 detected\n");
> + drm_info(dev, "AST 2300 detected\n");
> } else if (dev->pdev->revision >= 0x10) {
> switch (scu_rev & 0x0300) {
> case 0x0200:
> ast->chip = AST1100;
> - DRM_INFO("AST 1100 detected\n");
> + drm_info(dev, "AST 1100 detected\n");
> break;
> case 0x0100:
> ast->chip = AST2200;
> - DRM_INFO("AST 2200 detected\n");
> + drm_info(dev, "AST 2200 detected\n");
> break;
> case 0x0000:
> ast->chip = AST2150;
> - DRM_INFO("AST 2150 detected\n");
> + drm_info(dev, "AST 2150 detected\n");
> break;
> default:
> ast->chip = AST2100;
> - DRM_INFO("AST 2100 detected\n");
> + drm_info(dev, "AST 2100 detected\n");
> break;
> }
> ast->vga2_clone = false;
> } else {
> ast->chip = AST2000;
> - DRM_INFO("AST 2000 detected\n");
> + drm_info(dev, "AST 2000 detected\n");
> }
>
> /* Check if we support wide screen */
> @@ -248,13 +248,13 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> /* Print stuff for diagnostic purposes */
> switch(ast->tx_chip_type) {
> case AST_TX_SIL164:
> - DRM_INFO("Using Sil164 TMDS transmitter\n");
> + drm_info(dev, "Using Sil164 TMDS transmitter\n");
> break;
> case AST_TX_DP501:
> - DRM_INFO("Using DP501 DisplayPort transmitter\n");
> + drm_info(dev, "Using DP501 DisplayPort transmitter\n");
> break;
> default:
> - DRM_INFO("Analog VGA only\n");
> + drm_info(dev, "Analog VGA only\n");
> }
> return 0;
> }
> @@ -443,7 +443,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
> * and higher).
> */
> if (!(pci_resource_flags(dev->pdev, 2) & IORESOURCE_IO)) {
> - DRM_INFO("platform has no IO space, trying MMIO\n");
> + drm_info(dev, "platform has no IO space, trying MMIO\n");
> ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
> }
>
> @@ -465,7 +465,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
> if (ret)
> goto out_free;
> ast->vram_size = ast_get_vram_info(dev);
> - DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
> + drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
> ast->mclk, ast->dram_type,
> ast->dram_bus_width, ast->vram_size);
>
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 10211751182da..ff789f2db9fc8 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1104,7 +1104,7 @@ static int ast_connector_init(struct drm_device *dev)
> connector = &ast_connector->base;
> ast_connector->i2c = ast_i2c_create(dev);
> if (!ast_connector->i2c)
> - DRM_ERROR("failed to add ddc bus for connector\n");
> + drm_err(dev, "failed to add ddc bus for connector\n");
>
> drm_connector_init_with_ddc(dev, connector,
> &ast_connector_funcs,
> @@ -1188,7 +1188,7 @@ int ast_mode_init(struct drm_device *dev)
> ARRAY_SIZE(ast_primary_plane_formats),
> NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret) {
> - DRM_ERROR("ast: drm_universal_plane_init() failed: %d\n", ret);
> + drm_err(dev, "ast: drm_universal_plane_init() failed: %d\n", ret);
> return ret;
> }
> drm_plane_helper_add(&ast->primary_plane,
> @@ -1200,7 +1200,7 @@ int ast_mode_init(struct drm_device *dev)
> ARRAY_SIZE(ast_cursor_plane_formats),
> NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> if (ret) {
> - DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
> + drm_err(dev, "drm_universal_plane_failed(): %d\n", ret);
> return ret;
> }
> drm_plane_helper_add(&ast->cursor_plane,
> @@ -1322,7 +1322,7 @@ static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
> i2c->bit.getscl = get_clock;
> ret = i2c_bit_add_bus(&i2c->adapter);
> if (ret) {
> - DRM_ERROR("Failed to register bit i2c\n");
> + drm_err(dev, "Failed to register bit i2c\n");
> goto out_free;
> }
>
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 226e1290227ad..c043fe7175530 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2067,7 +2067,7 @@ void ast_post_chip_2500(struct drm_device *dev)
> }
>
> if (!ast_dram_init_2500(ast))
> - DRM_ERROR("DRAM init failed !\n");
> + drm_err(dev, "DRAM init failed !\n");
>
> temp = ast_mindwm(ast, 0x1e6e2040);
> ast_moutdwm(ast, 0x1e6e2040, temp | 0x40);
> diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
> index fad34106083a8..9c3788a4c1c54 100644
> --- a/drivers/gpu/drm/ast/ast_ttm.c
> +++ b/drivers/gpu/drm/ast/ast_ttm.c
> @@ -44,7 +44,7 @@ int ast_mm_init(struct ast_private *ast)
> ast->vram_size);
> if (IS_ERR(vmm)) {
> ret = PTR_ERR(vmm);
> - DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
> + drm_err(dev, "Error initializing VRAM MM; %d\n", ret);
> return ret;
> }
>
> --
> 2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] drm/ast: Use per-device logging macros
2020-06-11 19:24 ` Sam Ravnborg
@ 2020-06-12 6:28 ` Thomas Zimmermann
2020-06-12 7:01 ` Sam Ravnborg
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Zimmermann @ 2020-06-12 6:28 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: airlied, chen, kraxel, dri-devel, emil.velikov
[-- Attachment #1.1.1: Type: text/plain, Size: 8631 bytes --]
Hi Sam
Am 11.06.20 um 21:24 schrieb Sam Ravnborg:
> Hi Thomas.
> On Thu, Jun 11, 2020 at 10:28:09AM +0200, Thomas Zimmermann wrote:
>> Converts the ast driver to drm_info() and drm_err(). No functional
>> changes are made.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> I hope you will later follow-up with a patch that introduces drm_WARN_*.
I only found DRM_INFO and DRM_ERROR calls. Did I miss any other warning
macros?
Best regards
Thomas
>
> Sam
>
>> ---
>> drivers/gpu/drm/ast/ast_main.c | 34 +++++++++++++++++-----------------
>> drivers/gpu/drm/ast/ast_mode.c | 8 ++++----
>> drivers/gpu/drm/ast/ast_post.c | 2 +-
>> drivers/gpu/drm/ast/ast_ttm.c | 2 +-
>> 4 files changed, 23 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
>> index a2ef3d9077671..9063fdc9e8852 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -79,7 +79,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>> scu_rev)) {
>> /* We do, disable P2A access */
>> ast->config_mode = ast_use_dt;
>> - DRM_INFO("Using device-tree for configuration\n");
>> + drm_info(dev, "Using device-tree for configuration\n");
>> return;
>> }
>>
>> @@ -101,7 +101,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>> /* P2A works, grab silicon revision */
>> ast->config_mode = ast_use_p2a;
>>
>> - DRM_INFO("Using P2A bridge for configuration\n");
>> + drm_info(dev, "Using P2A bridge for configuration\n");
>>
>> /* Read SCU7c (silicon revision register) */
>> ast_write32(ast, 0xf004, 0x1e6e0000);
>> @@ -112,7 +112,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>> }
>>
>> /* We have a P2A bridge but it's disabled */
>> - DRM_INFO("P2A bridge disabled, using default configuration\n");
>> + drm_info(dev, "P2A bridge disabled, using default configuration\n");
>> }
>>
>> static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>> @@ -128,7 +128,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>> */
>> if (!ast_is_vga_enabled(dev)) {
>> ast_enable_vga(dev);
>> - DRM_INFO("VGA not enabled on entry, requesting chip POST\n");
>> + drm_info(dev, "VGA not enabled on entry, requesting chip POST\n");
>> *need_post = true;
>> } else
>> *need_post = false;
>> @@ -144,36 +144,36 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>> /* Identify chipset */
>> if (dev->pdev->revision >= 0x40) {
>> ast->chip = AST2500;
>> - DRM_INFO("AST 2500 detected\n");
>> + drm_info(dev, "AST 2500 detected\n");
>> } else if (dev->pdev->revision >= 0x30) {
>> ast->chip = AST2400;
>> - DRM_INFO("AST 2400 detected\n");
>> + drm_info(dev, "AST 2400 detected\n");
>> } else if (dev->pdev->revision >= 0x20) {
>> ast->chip = AST2300;
>> - DRM_INFO("AST 2300 detected\n");
>> + drm_info(dev, "AST 2300 detected\n");
>> } else if (dev->pdev->revision >= 0x10) {
>> switch (scu_rev & 0x0300) {
>> case 0x0200:
>> ast->chip = AST1100;
>> - DRM_INFO("AST 1100 detected\n");
>> + drm_info(dev, "AST 1100 detected\n");
>> break;
>> case 0x0100:
>> ast->chip = AST2200;
>> - DRM_INFO("AST 2200 detected\n");
>> + drm_info(dev, "AST 2200 detected\n");
>> break;
>> case 0x0000:
>> ast->chip = AST2150;
>> - DRM_INFO("AST 2150 detected\n");
>> + drm_info(dev, "AST 2150 detected\n");
>> break;
>> default:
>> ast->chip = AST2100;
>> - DRM_INFO("AST 2100 detected\n");
>> + drm_info(dev, "AST 2100 detected\n");
>> break;
>> }
>> ast->vga2_clone = false;
>> } else {
>> ast->chip = AST2000;
>> - DRM_INFO("AST 2000 detected\n");
>> + drm_info(dev, "AST 2000 detected\n");
>> }
>>
>> /* Check if we support wide screen */
>> @@ -248,13 +248,13 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>> /* Print stuff for diagnostic purposes */
>> switch(ast->tx_chip_type) {
>> case AST_TX_SIL164:
>> - DRM_INFO("Using Sil164 TMDS transmitter\n");
>> + drm_info(dev, "Using Sil164 TMDS transmitter\n");
>> break;
>> case AST_TX_DP501:
>> - DRM_INFO("Using DP501 DisplayPort transmitter\n");
>> + drm_info(dev, "Using DP501 DisplayPort transmitter\n");
>> break;
>> default:
>> - DRM_INFO("Analog VGA only\n");
>> + drm_info(dev, "Analog VGA only\n");
>> }
>> return 0;
>> }
>> @@ -443,7 +443,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>> * and higher).
>> */
>> if (!(pci_resource_flags(dev->pdev, 2) & IORESOURCE_IO)) {
>> - DRM_INFO("platform has no IO space, trying MMIO\n");
>> + drm_info(dev, "platform has no IO space, trying MMIO\n");
>> ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
>> }
>>
>> @@ -465,7 +465,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>> if (ret)
>> goto out_free;
>> ast->vram_size = ast_get_vram_info(dev);
>> - DRM_INFO("dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
>> + drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d size=%08x\n",
>> ast->mclk, ast->dram_type,
>> ast->dram_bus_width, ast->vram_size);
>>
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index 10211751182da..ff789f2db9fc8 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -1104,7 +1104,7 @@ static int ast_connector_init(struct drm_device *dev)
>> connector = &ast_connector->base;
>> ast_connector->i2c = ast_i2c_create(dev);
>> if (!ast_connector->i2c)
>> - DRM_ERROR("failed to add ddc bus for connector\n");
>> + drm_err(dev, "failed to add ddc bus for connector\n");
>>
>> drm_connector_init_with_ddc(dev, connector,
>> &ast_connector_funcs,
>> @@ -1188,7 +1188,7 @@ int ast_mode_init(struct drm_device *dev)
>> ARRAY_SIZE(ast_primary_plane_formats),
>> NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
>> if (ret) {
>> - DRM_ERROR("ast: drm_universal_plane_init() failed: %d\n", ret);
>> + drm_err(dev, "ast: drm_universal_plane_init() failed: %d\n", ret);
>> return ret;
>> }
>> drm_plane_helper_add(&ast->primary_plane,
>> @@ -1200,7 +1200,7 @@ int ast_mode_init(struct drm_device *dev)
>> ARRAY_SIZE(ast_cursor_plane_formats),
>> NULL, DRM_PLANE_TYPE_CURSOR, NULL);
>> if (ret) {
>> - DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
>> + drm_err(dev, "drm_universal_plane_failed(): %d\n", ret);
>> return ret;
>> }
>> drm_plane_helper_add(&ast->cursor_plane,
>> @@ -1322,7 +1322,7 @@ static struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev)
>> i2c->bit.getscl = get_clock;
>> ret = i2c_bit_add_bus(&i2c->adapter);
>> if (ret) {
>> - DRM_ERROR("Failed to register bit i2c\n");
>> + drm_err(dev, "Failed to register bit i2c\n");
>> goto out_free;
>> }
>>
>> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
>> index 226e1290227ad..c043fe7175530 100644
>> --- a/drivers/gpu/drm/ast/ast_post.c
>> +++ b/drivers/gpu/drm/ast/ast_post.c
>> @@ -2067,7 +2067,7 @@ void ast_post_chip_2500(struct drm_device *dev)
>> }
>>
>> if (!ast_dram_init_2500(ast))
>> - DRM_ERROR("DRAM init failed !\n");
>> + drm_err(dev, "DRAM init failed !\n");
>>
>> temp = ast_mindwm(ast, 0x1e6e2040);
>> ast_moutdwm(ast, 0x1e6e2040, temp | 0x40);
>> diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
>> index fad34106083a8..9c3788a4c1c54 100644
>> --- a/drivers/gpu/drm/ast/ast_ttm.c
>> +++ b/drivers/gpu/drm/ast/ast_ttm.c
>> @@ -44,7 +44,7 @@ int ast_mm_init(struct ast_private *ast)
>> ast->vram_size);
>> if (IS_ERR(vmm)) {
>> ret = PTR_ERR(vmm);
>> - DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
>> + drm_err(dev, "Error initializing VRAM MM; %d\n", ret);
>> return ret;
>> }
>>
>> --
>> 2.26.2
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private()
2020-06-11 19:23 ` Sam Ravnborg
@ 2020-06-12 6:30 ` Thomas Zimmermann
2020-06-12 6:57 ` Daniel Vetter
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Zimmermann @ 2020-06-12 6:30 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: airlied, chen, kraxel, dri-devel, emil.velikov
[-- Attachment #1.1.1: Type: text/plain, Size: 19128 bytes --]
Hi
Am 11.06.20 um 21:23 schrieb Sam Ravnborg:
> Hi Thomas.
>
> On Thu, Jun 11, 2020 at 10:28:08AM +0200, Thomas Zimmermann wrote:
>> All upcasting from struct drm_device to struct ast_private is now
>> performed via to_ast_private(). Using struct drm_device.dev_private is
>> deprecated.
>
> This is a simple 1:1 conversion.
> But some cases - I checked ast_set_dp501_video_output() =>
> ast_write_cmd(), ast_write_data()
>
> could have been fixed by passing ast_private * rather than drm_device *.
> And then no upcasting was needed.
>
> That a more involved approach - but wanted to point it out.
> Maybe for another day..
Good idea. I'll consider it, depending on whether it makes the overall
patchset easier or harder to read.
Best regards
Thomas
>
> Sam
>
>
> The ast variable in ast_crtc_helperatomic_check() is unused,
> s/ast_crtc_helperatomic_check/ast_crtc_helper_atomic_check/
>> so removed it.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
>> ---
>> drivers/gpu/drm/ast/ast_dp501.c | 24 +++++++++----------
>> drivers/gpu/drm/ast/ast_drv.h | 5 ++++
>> drivers/gpu/drm/ast/ast_main.c | 10 ++++----
>> drivers/gpu/drm/ast/ast_mode.c | 41 ++++++++++++++++-----------------
>> drivers/gpu/drm/ast/ast_post.c | 16 ++++++-------
>> 5 files changed, 50 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
>> index 98cd69269263f..4b85a504825a2 100644
>> --- a/drivers/gpu/drm/ast/ast_dp501.c
>> +++ b/drivers/gpu/drm/ast/ast_dp501.c
>> @@ -10,7 +10,7 @@ MODULE_FIRMWARE("ast_dp501_fw.bin");
>>
>> static int ast_load_dp501_microcode(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> return request_firmware(&ast->dp501_fw, "ast_dp501_fw.bin", dev->dev);
>> }
>> @@ -93,7 +93,7 @@ static bool wait_fw_ready(struct ast_private *ast)
>>
>> static bool ast_write_cmd(struct drm_device *dev, u8 data)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> int retry = 0;
>> if (wait_nack(ast)) {
>> send_nack(ast);
>> @@ -115,7 +115,7 @@ static bool ast_write_cmd(struct drm_device *dev, u8 data)
>>
>> static bool ast_write_data(struct drm_device *dev, u8 data)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> if (wait_nack(ast)) {
>> send_nack(ast);
>> @@ -133,7 +133,7 @@ static bool ast_write_data(struct drm_device *dev, u8 data)
>> #if 0
>> static bool ast_read_data(struct drm_device *dev, u8 *data)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 tmp;
>>
>> *data = 0;
>> @@ -172,7 +172,7 @@ static u32 get_fw_base(struct ast_private *ast)
>>
>> bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 i, data;
>> u32 boot_address;
>>
>> @@ -188,7 +188,7 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>>
>> static bool ast_launch_m68k(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 i, data, len = 0;
>> u32 boot_address;
>> u8 *fw_addr = NULL;
>> @@ -255,7 +255,7 @@ static bool ast_launch_m68k(struct drm_device *dev)
>>
>> u8 ast_get_dp501_max_clk(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 boot_address, offset, data;
>> u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
>>
>> @@ -283,7 +283,7 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>>
>> bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 i, boot_address, offset, data;
>>
>> boot_address = get_fw_base(ast);
>> @@ -312,7 +312,7 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>>
>> static bool ast_init_dvo(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 jreg;
>> u32 data;
>> ast_write32(ast, 0xf004, 0x1e6e0000);
>> @@ -385,7 +385,7 @@ static bool ast_init_dvo(struct drm_device *dev)
>>
>> static void ast_init_analog(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 data;
>>
>> /*
>> @@ -412,7 +412,7 @@ static void ast_init_analog(struct drm_device *dev)
>>
>> void ast_init_3rdtx(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 jreg;
>>
>> if (ast->chip == AST2300 || ast->chip == AST2400) {
>> @@ -438,7 +438,7 @@ void ast_init_3rdtx(struct drm_device *dev)
>>
>> void ast_release_firmware(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> release_firmware(ast->dp501_fw);
>> ast->dp501_fw = NULL;
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
>> index 09f2659e29118..c44c1376c6977 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -136,6 +136,11 @@ struct ast_private {
>> const struct firmware *dp501_fw; /* dp501 fw */
>> };
>>
>> +static inline struct ast_private *to_ast_private(struct drm_device *dev)
>> +{
>> + return dev->dev_private;
>> +}
>> +
>> int ast_driver_load(struct drm_device *dev, unsigned long flags);
>> void ast_driver_unload(struct drm_device *dev);
>>
>> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
>> index f48a9f62368c0..a2ef3d9077671 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -67,7 +67,7 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
>> static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>> {
>> struct device_node *np = dev->pdev->dev.of_node;
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> uint32_t data, jregd0, jregd1;
>>
>> /* Defaults */
>> @@ -117,7 +117,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>>
>> static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> uint32_t jreg, scu_rev;
>>
>> /*
>> @@ -262,7 +262,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>> static int ast_get_dram_info(struct drm_device *dev)
>> {
>> struct device_node *np = dev->pdev->dev.of_node;
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
>> uint32_t denum, num, div, ref_pll, dsel;
>>
>> @@ -388,7 +388,7 @@ static const struct drm_mode_config_funcs ast_mode_funcs = {
>>
>> static u32 ast_get_vram_info(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 jreg;
>> u32 vram_size;
>> ast_open_key(ast);
>> @@ -509,7 +509,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>>
>> void ast_driver_unload(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> /* enable standard VGA decode */
>> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index be0e2250708fa..10211751182da 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -565,7 +565,7 @@ static void
>> ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
>> struct drm_plane_state *old_state)
>> {
>> - struct ast_private *ast = plane->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(plane->dev);
>> struct drm_plane_state *state = plane->state;
>> struct drm_gem_vram_object *gbo;
>> s64 gpu_addr;
>> @@ -585,7 +585,7 @@ static void
>> ast_primary_plane_helper_atomic_disable(struct drm_plane *plane,
>> struct drm_plane_state *old_state)
>> {
>> - struct ast_private *ast = plane->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(plane->dev);
>>
>> ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x20);
>> }
>> @@ -633,7 +633,7 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
>> WARN_ON_ONCE(fb->height > AST_MAX_HWC_HEIGHT))
>> return -EINVAL; /* BUG: didn't test in atomic_check() */
>>
>> - ast = crtc->dev->dev_private;
>> + ast = to_ast_private(crtc->dev);
>>
>> gbo = drm_gem_vram_of_gem(fb->obj[0]);
>> src = drm_gem_vram_vmap(gbo);
>> @@ -705,7 +705,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
>> struct drm_plane_state *state = plane->state;
>> struct drm_crtc *crtc = state->crtc;
>> struct drm_framebuffer *fb = state->fb;
>> - struct ast_private *ast = plane->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(plane->dev);
>> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
>> struct drm_gem_vram_object *gbo;
>> s64 off;
>> @@ -738,7 +738,7 @@ static void
>> ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
>> struct drm_plane_state *old_state)
>> {
>> - struct ast_private *ast = plane->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(plane->dev);
>>
>> ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
>> }
>> @@ -766,7 +766,7 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs = {
>>
>> static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
>> {
>> - struct ast_private *ast = crtc->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(crtc->dev);
>>
>> /* TODO: Maybe control display signal generation with
>> * Sync Enable (bit CR17.7).
>> @@ -789,7 +789,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
>> static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
>> struct drm_crtc_state *state)
>> {
>> - struct ast_private *ast = crtc->dev->dev_private;
>> struct ast_crtc_state *ast_state;
>> const struct drm_format_info *format;
>> bool succ;
>> @@ -815,7 +814,7 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
>> static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc,
>> struct drm_crtc_state *old_crtc_state)
>> {
>> - struct ast_private *ast = crtc->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(crtc->dev);
>>
>> ast_open_key(ast);
>> }
>> @@ -824,7 +823,7 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
>> struct drm_crtc_state *old_crtc_state)
>> {
>> struct drm_device *dev = crtc->dev;
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> struct ast_crtc_state *ast_state;
>> const struct drm_format_info *format;
>> struct ast_vbios_mode_info *vbios_mode_info;
>> @@ -937,7 +936,7 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
>>
>> static int ast_crtc_init(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> struct ast_crtc *crtc;
>> int ret;
>>
>> @@ -966,7 +965,7 @@ static int ast_crtc_init(struct drm_device *dev)
>>
>> static int ast_encoder_init(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> struct drm_encoder *encoder = &ast->encoder;
>> int ret;
>>
>> @@ -986,7 +985,7 @@ static int ast_encoder_init(struct drm_device *dev)
>> static int ast_get_modes(struct drm_connector *connector)
>> {
>> struct ast_connector *ast_connector = to_ast_connector(connector);
>> - struct ast_private *ast = connector->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(connector->dev);
>> struct edid *edid;
>> int ret;
>> bool flags = false;
>> @@ -1017,7 +1016,7 @@ static int ast_get_modes(struct drm_connector *connector)
>> static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
>> struct drm_display_mode *mode)
>> {
>> - struct ast_private *ast = connector->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(connector->dev);
>> int flags = MODE_NOMODE;
>> uint32_t jtemp;
>>
>> @@ -1128,7 +1127,7 @@ static int ast_connector_init(struct drm_device *dev)
>> /* allocate cursor cache and pin at start of VRAM */
>> static int ast_cursor_init(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> size_t size, i;
>> struct drm_gem_vram_object *gbo;
>> int ret;
>> @@ -1166,7 +1165,7 @@ static int ast_cursor_init(struct drm_device *dev)
>>
>> static void ast_cursor_fini(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> size_t i;
>> struct drm_gem_vram_object *gbo;
>>
>> @@ -1179,7 +1178,7 @@ static void ast_cursor_fini(struct drm_device *dev)
>>
>> int ast_mode_init(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> int ret;
>>
>> memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
>> @@ -1223,7 +1222,7 @@ void ast_mode_fini(struct drm_device *dev)
>> static int get_clock(void *i2c_priv)
>> {
>> struct ast_i2c_chan *i2c = i2c_priv;
>> - struct ast_private *ast = i2c->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(i2c->dev);
>> uint32_t val, val2, count, pass;
>>
>> count = 0;
>> @@ -1245,7 +1244,7 @@ static int get_clock(void *i2c_priv)
>> static int get_data(void *i2c_priv)
>> {
>> struct ast_i2c_chan *i2c = i2c_priv;
>> - struct ast_private *ast = i2c->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(i2c->dev);
>> uint32_t val, val2, count, pass;
>>
>> count = 0;
>> @@ -1267,7 +1266,7 @@ static int get_data(void *i2c_priv)
>> static void set_clock(void *i2c_priv, int clock)
>> {
>> struct ast_i2c_chan *i2c = i2c_priv;
>> - struct ast_private *ast = i2c->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(i2c->dev);
>> int i;
>> u8 ujcrb7, jtemp;
>>
>> @@ -1283,7 +1282,7 @@ static void set_clock(void *i2c_priv, int clock)
>> static void set_data(void *i2c_priv, int data)
>> {
>> struct ast_i2c_chan *i2c = i2c_priv;
>> - struct ast_private *ast = i2c->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(i2c->dev);
>> int i;
>> u8 ujcrb7, jtemp;
>>
>> @@ -1431,7 +1430,7 @@ static int ast_cursor_move(struct drm_crtc *crtc,
>> int x, int y)
>> {
>> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
>> - struct ast_private *ast = crtc->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(crtc->dev);
>> struct drm_gem_vram_object *gbo;
>> int x_offset, y_offset;
>> u8 *dst, *sig;
>> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
>> index af0c8ebb009a1..226e1290227ad 100644
>> --- a/drivers/gpu/drm/ast/ast_post.c
>> +++ b/drivers/gpu/drm/ast/ast_post.c
>> @@ -39,7 +39,7 @@ static void ast_post_chip_2500(struct drm_device *dev);
>>
>> void ast_enable_vga(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> ast_io_write8(ast, AST_IO_VGA_ENABLE_PORT, 0x01);
>> ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
>> @@ -47,7 +47,7 @@ void ast_enable_vga(struct drm_device *dev)
>>
>> void ast_enable_mmio(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
>> }
>> @@ -55,7 +55,7 @@ void ast_enable_mmio(struct drm_device *dev)
>>
>> bool ast_is_vga_enabled(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 ch;
>>
>> ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
>> @@ -70,7 +70,7 @@ static const u8 extreginfo_ast2300[] = { 0x0f, 0x04, 0x1f, 0xff };
>> static void
>> ast_set_def_ext_reg(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 i, index, reg;
>> const u8 *ext_reg_info;
>>
>> @@ -272,7 +272,7 @@ static void cbrdlli_ast2150(struct ast_private *ast, int busw)
>>
>> static void ast_init_dram_reg(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 j;
>> u32 data, temp, i;
>> const struct ast_dramstruct *dram_reg_info;
>> @@ -366,7 +366,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
>> void ast_post_gpu(struct drm_device *dev)
>> {
>> u32 reg;
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> pci_read_config_dword(ast->dev->pdev, 0x04, ®);
>> reg |= 0x3;
>> @@ -1596,7 +1596,7 @@ static void ddr2_init(struct ast_private *ast, struct ast2300_dram_param *param)
>>
>> static void ast_post_chip_2300(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> struct ast2300_dram_param param;
>> u32 temp;
>> u8 reg;
>> @@ -2028,7 +2028,7 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>>
>> void ast_post_chip_2500(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 temp;
>> u8 reg;
>>
>> --
>> 2.26.2
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private()
2020-06-11 17:32 ` Daniel Vetter
@ 2020-06-12 6:31 ` Thomas Zimmermann
0 siblings, 0 replies; 17+ messages in thread
From: Thomas Zimmermann @ 2020-06-12 6:31 UTC (permalink / raw)
To: Daniel Vetter; +Cc: chen, dri-devel, kraxel, airlied, sam, emil.velikov
[-- Attachment #1.1.1: Type: text/plain, Size: 18989 bytes --]
Hi
Am 11.06.20 um 19:32 schrieb Daniel Vetter:
> On Thu, Jun 11, 2020 at 10:28:08AM +0200, Thomas Zimmermann wrote:
>> All upcasting from struct drm_device to struct ast_private is now
>> performed via to_ast_private(). Using struct drm_device.dev_private is
>> deprecated. The ast variable in ast_crtc_helperatomic_check() is unused,
>> so removed it.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Aside, the check in ast_pm_freeze is bogus, you can't resume/freeze before
> the driver has completed loading.
Ah, OK. I'll remove it then.
Best regards
Thomas
>
> I suspect this is a remnant from the old days of dri1 where freeze/resume
> before the driver finished loading was very much possible with the shadow
> attach stuff. So probably just copypasta stuff.
>
> In general when you spot that in a modern kms driver, then just delete it.
> that = checking whether the drm_device or dev_private is set. Definitely
> not a pattern we should propagate.
>
> Cheers, Daniel
>
>> ---
>> drivers/gpu/drm/ast/ast_dp501.c | 24 +++++++++----------
>> drivers/gpu/drm/ast/ast_drv.h | 5 ++++
>> drivers/gpu/drm/ast/ast_main.c | 10 ++++----
>> drivers/gpu/drm/ast/ast_mode.c | 41 ++++++++++++++++-----------------
>> drivers/gpu/drm/ast/ast_post.c | 16 ++++++-------
>> 5 files changed, 50 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
>> index 98cd69269263f..4b85a504825a2 100644
>> --- a/drivers/gpu/drm/ast/ast_dp501.c
>> +++ b/drivers/gpu/drm/ast/ast_dp501.c
>> @@ -10,7 +10,7 @@ MODULE_FIRMWARE("ast_dp501_fw.bin");
>>
>> static int ast_load_dp501_microcode(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> return request_firmware(&ast->dp501_fw, "ast_dp501_fw.bin", dev->dev);
>> }
>> @@ -93,7 +93,7 @@ static bool wait_fw_ready(struct ast_private *ast)
>>
>> static bool ast_write_cmd(struct drm_device *dev, u8 data)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> int retry = 0;
>> if (wait_nack(ast)) {
>> send_nack(ast);
>> @@ -115,7 +115,7 @@ static bool ast_write_cmd(struct drm_device *dev, u8 data)
>>
>> static bool ast_write_data(struct drm_device *dev, u8 data)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> if (wait_nack(ast)) {
>> send_nack(ast);
>> @@ -133,7 +133,7 @@ static bool ast_write_data(struct drm_device *dev, u8 data)
>> #if 0
>> static bool ast_read_data(struct drm_device *dev, u8 *data)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 tmp;
>>
>> *data = 0;
>> @@ -172,7 +172,7 @@ static u32 get_fw_base(struct ast_private *ast)
>>
>> bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 i, data;
>> u32 boot_address;
>>
>> @@ -188,7 +188,7 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>>
>> static bool ast_launch_m68k(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 i, data, len = 0;
>> u32 boot_address;
>> u8 *fw_addr = NULL;
>> @@ -255,7 +255,7 @@ static bool ast_launch_m68k(struct drm_device *dev)
>>
>> u8 ast_get_dp501_max_clk(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 boot_address, offset, data;
>> u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
>>
>> @@ -283,7 +283,7 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>>
>> bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 i, boot_address, offset, data;
>>
>> boot_address = get_fw_base(ast);
>> @@ -312,7 +312,7 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>>
>> static bool ast_init_dvo(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 jreg;
>> u32 data;
>> ast_write32(ast, 0xf004, 0x1e6e0000);
>> @@ -385,7 +385,7 @@ static bool ast_init_dvo(struct drm_device *dev)
>>
>> static void ast_init_analog(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 data;
>>
>> /*
>> @@ -412,7 +412,7 @@ static void ast_init_analog(struct drm_device *dev)
>>
>> void ast_init_3rdtx(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 jreg;
>>
>> if (ast->chip == AST2300 || ast->chip == AST2400) {
>> @@ -438,7 +438,7 @@ void ast_init_3rdtx(struct drm_device *dev)
>>
>> void ast_release_firmware(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> release_firmware(ast->dp501_fw);
>> ast->dp501_fw = NULL;
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
>> index 09f2659e29118..c44c1376c6977 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -136,6 +136,11 @@ struct ast_private {
>> const struct firmware *dp501_fw; /* dp501 fw */
>> };
>>
>> +static inline struct ast_private *to_ast_private(struct drm_device *dev)
>> +{
>> + return dev->dev_private;
>> +}
>> +
>> int ast_driver_load(struct drm_device *dev, unsigned long flags);
>> void ast_driver_unload(struct drm_device *dev);
>>
>> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
>> index f48a9f62368c0..a2ef3d9077671 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -67,7 +67,7 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
>> static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>> {
>> struct device_node *np = dev->pdev->dev.of_node;
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> uint32_t data, jregd0, jregd1;
>>
>> /* Defaults */
>> @@ -117,7 +117,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>>
>> static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> uint32_t jreg, scu_rev;
>>
>> /*
>> @@ -262,7 +262,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>> static int ast_get_dram_info(struct drm_device *dev)
>> {
>> struct device_node *np = dev->pdev->dev.of_node;
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
>> uint32_t denum, num, div, ref_pll, dsel;
>>
>> @@ -388,7 +388,7 @@ static const struct drm_mode_config_funcs ast_mode_funcs = {
>>
>> static u32 ast_get_vram_info(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 jreg;
>> u32 vram_size;
>> ast_open_key(ast);
>> @@ -509,7 +509,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
>>
>> void ast_driver_unload(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> /* enable standard VGA decode */
>> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index be0e2250708fa..10211751182da 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -565,7 +565,7 @@ static void
>> ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
>> struct drm_plane_state *old_state)
>> {
>> - struct ast_private *ast = plane->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(plane->dev);
>> struct drm_plane_state *state = plane->state;
>> struct drm_gem_vram_object *gbo;
>> s64 gpu_addr;
>> @@ -585,7 +585,7 @@ static void
>> ast_primary_plane_helper_atomic_disable(struct drm_plane *plane,
>> struct drm_plane_state *old_state)
>> {
>> - struct ast_private *ast = plane->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(plane->dev);
>>
>> ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x20);
>> }
>> @@ -633,7 +633,7 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
>> WARN_ON_ONCE(fb->height > AST_MAX_HWC_HEIGHT))
>> return -EINVAL; /* BUG: didn't test in atomic_check() */
>>
>> - ast = crtc->dev->dev_private;
>> + ast = to_ast_private(crtc->dev);
>>
>> gbo = drm_gem_vram_of_gem(fb->obj[0]);
>> src = drm_gem_vram_vmap(gbo);
>> @@ -705,7 +705,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
>> struct drm_plane_state *state = plane->state;
>> struct drm_crtc *crtc = state->crtc;
>> struct drm_framebuffer *fb = state->fb;
>> - struct ast_private *ast = plane->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(plane->dev);
>> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
>> struct drm_gem_vram_object *gbo;
>> s64 off;
>> @@ -738,7 +738,7 @@ static void
>> ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
>> struct drm_plane_state *old_state)
>> {
>> - struct ast_private *ast = plane->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(plane->dev);
>>
>> ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
>> }
>> @@ -766,7 +766,7 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs = {
>>
>> static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
>> {
>> - struct ast_private *ast = crtc->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(crtc->dev);
>>
>> /* TODO: Maybe control display signal generation with
>> * Sync Enable (bit CR17.7).
>> @@ -789,7 +789,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
>> static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
>> struct drm_crtc_state *state)
>> {
>> - struct ast_private *ast = crtc->dev->dev_private;
>> struct ast_crtc_state *ast_state;
>> const struct drm_format_info *format;
>> bool succ;
>> @@ -815,7 +814,7 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
>> static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc,
>> struct drm_crtc_state *old_crtc_state)
>> {
>> - struct ast_private *ast = crtc->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(crtc->dev);
>>
>> ast_open_key(ast);
>> }
>> @@ -824,7 +823,7 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
>> struct drm_crtc_state *old_crtc_state)
>> {
>> struct drm_device *dev = crtc->dev;
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> struct ast_crtc_state *ast_state;
>> const struct drm_format_info *format;
>> struct ast_vbios_mode_info *vbios_mode_info;
>> @@ -937,7 +936,7 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
>>
>> static int ast_crtc_init(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> struct ast_crtc *crtc;
>> int ret;
>>
>> @@ -966,7 +965,7 @@ static int ast_crtc_init(struct drm_device *dev)
>>
>> static int ast_encoder_init(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> struct drm_encoder *encoder = &ast->encoder;
>> int ret;
>>
>> @@ -986,7 +985,7 @@ static int ast_encoder_init(struct drm_device *dev)
>> static int ast_get_modes(struct drm_connector *connector)
>> {
>> struct ast_connector *ast_connector = to_ast_connector(connector);
>> - struct ast_private *ast = connector->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(connector->dev);
>> struct edid *edid;
>> int ret;
>> bool flags = false;
>> @@ -1017,7 +1016,7 @@ static int ast_get_modes(struct drm_connector *connector)
>> static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
>> struct drm_display_mode *mode)
>> {
>> - struct ast_private *ast = connector->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(connector->dev);
>> int flags = MODE_NOMODE;
>> uint32_t jtemp;
>>
>> @@ -1128,7 +1127,7 @@ static int ast_connector_init(struct drm_device *dev)
>> /* allocate cursor cache and pin at start of VRAM */
>> static int ast_cursor_init(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> size_t size, i;
>> struct drm_gem_vram_object *gbo;
>> int ret;
>> @@ -1166,7 +1165,7 @@ static int ast_cursor_init(struct drm_device *dev)
>>
>> static void ast_cursor_fini(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> size_t i;
>> struct drm_gem_vram_object *gbo;
>>
>> @@ -1179,7 +1178,7 @@ static void ast_cursor_fini(struct drm_device *dev)
>>
>> int ast_mode_init(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> int ret;
>>
>> memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
>> @@ -1223,7 +1222,7 @@ void ast_mode_fini(struct drm_device *dev)
>> static int get_clock(void *i2c_priv)
>> {
>> struct ast_i2c_chan *i2c = i2c_priv;
>> - struct ast_private *ast = i2c->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(i2c->dev);
>> uint32_t val, val2, count, pass;
>>
>> count = 0;
>> @@ -1245,7 +1244,7 @@ static int get_clock(void *i2c_priv)
>> static int get_data(void *i2c_priv)
>> {
>> struct ast_i2c_chan *i2c = i2c_priv;
>> - struct ast_private *ast = i2c->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(i2c->dev);
>> uint32_t val, val2, count, pass;
>>
>> count = 0;
>> @@ -1267,7 +1266,7 @@ static int get_data(void *i2c_priv)
>> static void set_clock(void *i2c_priv, int clock)
>> {
>> struct ast_i2c_chan *i2c = i2c_priv;
>> - struct ast_private *ast = i2c->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(i2c->dev);
>> int i;
>> u8 ujcrb7, jtemp;
>>
>> @@ -1283,7 +1282,7 @@ static void set_clock(void *i2c_priv, int clock)
>> static void set_data(void *i2c_priv, int data)
>> {
>> struct ast_i2c_chan *i2c = i2c_priv;
>> - struct ast_private *ast = i2c->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(i2c->dev);
>> int i;
>> u8 ujcrb7, jtemp;
>>
>> @@ -1431,7 +1430,7 @@ static int ast_cursor_move(struct drm_crtc *crtc,
>> int x, int y)
>> {
>> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
>> - struct ast_private *ast = crtc->dev->dev_private;
>> + struct ast_private *ast = to_ast_private(crtc->dev);
>> struct drm_gem_vram_object *gbo;
>> int x_offset, y_offset;
>> u8 *dst, *sig;
>> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
>> index af0c8ebb009a1..226e1290227ad 100644
>> --- a/drivers/gpu/drm/ast/ast_post.c
>> +++ b/drivers/gpu/drm/ast/ast_post.c
>> @@ -39,7 +39,7 @@ static void ast_post_chip_2500(struct drm_device *dev);
>>
>> void ast_enable_vga(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> ast_io_write8(ast, AST_IO_VGA_ENABLE_PORT, 0x01);
>> ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
>> @@ -47,7 +47,7 @@ void ast_enable_vga(struct drm_device *dev)
>>
>> void ast_enable_mmio(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
>> }
>> @@ -55,7 +55,7 @@ void ast_enable_mmio(struct drm_device *dev)
>>
>> bool ast_is_vga_enabled(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 ch;
>>
>> ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
>> @@ -70,7 +70,7 @@ static const u8 extreginfo_ast2300[] = { 0x0f, 0x04, 0x1f, 0xff };
>> static void
>> ast_set_def_ext_reg(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 i, index, reg;
>> const u8 *ext_reg_info;
>>
>> @@ -272,7 +272,7 @@ static void cbrdlli_ast2150(struct ast_private *ast, int busw)
>>
>> static void ast_init_dram_reg(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u8 j;
>> u32 data, temp, i;
>> const struct ast_dramstruct *dram_reg_info;
>> @@ -366,7 +366,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
>> void ast_post_gpu(struct drm_device *dev)
>> {
>> u32 reg;
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>>
>> pci_read_config_dword(ast->dev->pdev, 0x04, ®);
>> reg |= 0x3;
>> @@ -1596,7 +1596,7 @@ static void ddr2_init(struct ast_private *ast, struct ast2300_dram_param *param)
>>
>> static void ast_post_chip_2300(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> struct ast2300_dram_param param;
>> u32 temp;
>> u8 reg;
>> @@ -2028,7 +2028,7 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>>
>> void ast_post_chip_2500(struct drm_device *dev)
>> {
>> - struct ast_private *ast = dev->dev_private;
>> + struct ast_private *ast = to_ast_private(dev);
>> u32 temp;
>> u8 reg;
>>
>> --
>> 2.26.2
>>
>
--
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: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private()
2020-06-12 6:30 ` Thomas Zimmermann
@ 2020-06-12 6:57 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2020-06-12 6:57 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: chen, dri-devel, kraxel, airlied, Sam Ravnborg, emil.velikov
On Fri, Jun 12, 2020 at 08:30:40AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 11.06.20 um 21:23 schrieb Sam Ravnborg:
> > Hi Thomas.
> >
> > On Thu, Jun 11, 2020 at 10:28:08AM +0200, Thomas Zimmermann wrote:
> >> All upcasting from struct drm_device to struct ast_private is now
> >> performed via to_ast_private(). Using struct drm_device.dev_private is
> >> deprecated.
> >
> > This is a simple 1:1 conversion.
> > But some cases - I checked ast_set_dp501_video_output() =>
> > ast_write_cmd(), ast_write_data()
> >
> > could have been fixed by passing ast_private * rather than drm_device *.
> > And then no upcasting was needed.
> >
> > That a more involved approach - but wanted to point it out.
> > Maybe for another day..
>
> Good idea. I'll consider it, depending on whether it makes the overall
> patchset easier or harder to read.
tbh since the goal is to embed the structures (I assume) I wouldn't
bother. As soon as it's embedded it's the same for the compiler. If you
flip some of the parameters I'd wait until the embedding is completely,
otherwise you have a pile of churn again because of the switch from
pointer to embedded structure.
-Daniel
>
> Best regards
> Thomas
>
> >
> > Sam
> >
> >
> > The ast variable in ast_crtc_helperatomic_check() is unused,
> > s/ast_crtc_helperatomic_check/ast_crtc_helper_atomic_check/
> >> so removed it.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> >
> >> ---
> >> drivers/gpu/drm/ast/ast_dp501.c | 24 +++++++++----------
> >> drivers/gpu/drm/ast/ast_drv.h | 5 ++++
> >> drivers/gpu/drm/ast/ast_main.c | 10 ++++----
> >> drivers/gpu/drm/ast/ast_mode.c | 41 ++++++++++++++++-----------------
> >> drivers/gpu/drm/ast/ast_post.c | 16 ++++++-------
> >> 5 files changed, 50 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
> >> index 98cd69269263f..4b85a504825a2 100644
> >> --- a/drivers/gpu/drm/ast/ast_dp501.c
> >> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> >> @@ -10,7 +10,7 @@ MODULE_FIRMWARE("ast_dp501_fw.bin");
> >>
> >> static int ast_load_dp501_microcode(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >>
> >> return request_firmware(&ast->dp501_fw, "ast_dp501_fw.bin", dev->dev);
> >> }
> >> @@ -93,7 +93,7 @@ static bool wait_fw_ready(struct ast_private *ast)
> >>
> >> static bool ast_write_cmd(struct drm_device *dev, u8 data)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> int retry = 0;
> >> if (wait_nack(ast)) {
> >> send_nack(ast);
> >> @@ -115,7 +115,7 @@ static bool ast_write_cmd(struct drm_device *dev, u8 data)
> >>
> >> static bool ast_write_data(struct drm_device *dev, u8 data)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >>
> >> if (wait_nack(ast)) {
> >> send_nack(ast);
> >> @@ -133,7 +133,7 @@ static bool ast_write_data(struct drm_device *dev, u8 data)
> >> #if 0
> >> static bool ast_read_data(struct drm_device *dev, u8 *data)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u8 tmp;
> >>
> >> *data = 0;
> >> @@ -172,7 +172,7 @@ static u32 get_fw_base(struct ast_private *ast)
> >>
> >> bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u32 i, data;
> >> u32 boot_address;
> >>
> >> @@ -188,7 +188,7 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
> >>
> >> static bool ast_launch_m68k(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u32 i, data, len = 0;
> >> u32 boot_address;
> >> u8 *fw_addr = NULL;
> >> @@ -255,7 +255,7 @@ static bool ast_launch_m68k(struct drm_device *dev)
> >>
> >> u8 ast_get_dp501_max_clk(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u32 boot_address, offset, data;
> >> u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
> >>
> >> @@ -283,7 +283,7 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
> >>
> >> bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u32 i, boot_address, offset, data;
> >>
> >> boot_address = get_fw_base(ast);
> >> @@ -312,7 +312,7 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
> >>
> >> static bool ast_init_dvo(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u8 jreg;
> >> u32 data;
> >> ast_write32(ast, 0xf004, 0x1e6e0000);
> >> @@ -385,7 +385,7 @@ static bool ast_init_dvo(struct drm_device *dev)
> >>
> >> static void ast_init_analog(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u32 data;
> >>
> >> /*
> >> @@ -412,7 +412,7 @@ static void ast_init_analog(struct drm_device *dev)
> >>
> >> void ast_init_3rdtx(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u8 jreg;
> >>
> >> if (ast->chip == AST2300 || ast->chip == AST2400) {
> >> @@ -438,7 +438,7 @@ void ast_init_3rdtx(struct drm_device *dev)
> >>
> >> void ast_release_firmware(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >>
> >> release_firmware(ast->dp501_fw);
> >> ast->dp501_fw = NULL;
> >> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> >> index 09f2659e29118..c44c1376c6977 100644
> >> --- a/drivers/gpu/drm/ast/ast_drv.h
> >> +++ b/drivers/gpu/drm/ast/ast_drv.h
> >> @@ -136,6 +136,11 @@ struct ast_private {
> >> const struct firmware *dp501_fw; /* dp501 fw */
> >> };
> >>
> >> +static inline struct ast_private *to_ast_private(struct drm_device *dev)
> >> +{
> >> + return dev->dev_private;
> >> +}
> >> +
> >> int ast_driver_load(struct drm_device *dev, unsigned long flags);
> >> void ast_driver_unload(struct drm_device *dev);
> >>
> >> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> >> index f48a9f62368c0..a2ef3d9077671 100644
> >> --- a/drivers/gpu/drm/ast/ast_main.c
> >> +++ b/drivers/gpu/drm/ast/ast_main.c
> >> @@ -67,7 +67,7 @@ uint8_t ast_get_index_reg_mask(struct ast_private *ast,
> >> static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> >> {
> >> struct device_node *np = dev->pdev->dev.of_node;
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> uint32_t data, jregd0, jregd1;
> >>
> >> /* Defaults */
> >> @@ -117,7 +117,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> >>
> >> static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> uint32_t jreg, scu_rev;
> >>
> >> /*
> >> @@ -262,7 +262,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
> >> static int ast_get_dram_info(struct drm_device *dev)
> >> {
> >> struct device_node *np = dev->pdev->dev.of_node;
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
> >> uint32_t denum, num, div, ref_pll, dsel;
> >>
> >> @@ -388,7 +388,7 @@ static const struct drm_mode_config_funcs ast_mode_funcs = {
> >>
> >> static u32 ast_get_vram_info(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u8 jreg;
> >> u32 vram_size;
> >> ast_open_key(ast);
> >> @@ -509,7 +509,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
> >>
> >> void ast_driver_unload(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >>
> >> /* enable standard VGA decode */
> >> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
> >> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> >> index be0e2250708fa..10211751182da 100644
> >> --- a/drivers/gpu/drm/ast/ast_mode.c
> >> +++ b/drivers/gpu/drm/ast/ast_mode.c
> >> @@ -565,7 +565,7 @@ static void
> >> ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
> >> struct drm_plane_state *old_state)
> >> {
> >> - struct ast_private *ast = plane->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(plane->dev);
> >> struct drm_plane_state *state = plane->state;
> >> struct drm_gem_vram_object *gbo;
> >> s64 gpu_addr;
> >> @@ -585,7 +585,7 @@ static void
> >> ast_primary_plane_helper_atomic_disable(struct drm_plane *plane,
> >> struct drm_plane_state *old_state)
> >> {
> >> - struct ast_private *ast = plane->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(plane->dev);
> >>
> >> ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x20);
> >> }
> >> @@ -633,7 +633,7 @@ ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
> >> WARN_ON_ONCE(fb->height > AST_MAX_HWC_HEIGHT))
> >> return -EINVAL; /* BUG: didn't test in atomic_check() */
> >>
> >> - ast = crtc->dev->dev_private;
> >> + ast = to_ast_private(crtc->dev);
> >>
> >> gbo = drm_gem_vram_of_gem(fb->obj[0]);
> >> src = drm_gem_vram_vmap(gbo);
> >> @@ -705,7 +705,7 @@ ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
> >> struct drm_plane_state *state = plane->state;
> >> struct drm_crtc *crtc = state->crtc;
> >> struct drm_framebuffer *fb = state->fb;
> >> - struct ast_private *ast = plane->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(plane->dev);
> >> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> >> struct drm_gem_vram_object *gbo;
> >> s64 off;
> >> @@ -738,7 +738,7 @@ static void
> >> ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
> >> struct drm_plane_state *old_state)
> >> {
> >> - struct ast_private *ast = plane->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(plane->dev);
> >>
> >> ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
> >> }
> >> @@ -766,7 +766,7 @@ static const struct drm_plane_funcs ast_cursor_plane_funcs = {
> >>
> >> static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
> >> {
> >> - struct ast_private *ast = crtc->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(crtc->dev);
> >>
> >> /* TODO: Maybe control display signal generation with
> >> * Sync Enable (bit CR17.7).
> >> @@ -789,7 +789,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
> >> static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
> >> struct drm_crtc_state *state)
> >> {
> >> - struct ast_private *ast = crtc->dev->dev_private;
> >> struct ast_crtc_state *ast_state;
> >> const struct drm_format_info *format;
> >> bool succ;
> >> @@ -815,7 +814,7 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
> >> static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc,
> >> struct drm_crtc_state *old_crtc_state)
> >> {
> >> - struct ast_private *ast = crtc->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(crtc->dev);
> >>
> >> ast_open_key(ast);
> >> }
> >> @@ -824,7 +823,7 @@ static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc,
> >> struct drm_crtc_state *old_crtc_state)
> >> {
> >> struct drm_device *dev = crtc->dev;
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> struct ast_crtc_state *ast_state;
> >> const struct drm_format_info *format;
> >> struct ast_vbios_mode_info *vbios_mode_info;
> >> @@ -937,7 +936,7 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
> >>
> >> static int ast_crtc_init(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> struct ast_crtc *crtc;
> >> int ret;
> >>
> >> @@ -966,7 +965,7 @@ static int ast_crtc_init(struct drm_device *dev)
> >>
> >> static int ast_encoder_init(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> struct drm_encoder *encoder = &ast->encoder;
> >> int ret;
> >>
> >> @@ -986,7 +985,7 @@ static int ast_encoder_init(struct drm_device *dev)
> >> static int ast_get_modes(struct drm_connector *connector)
> >> {
> >> struct ast_connector *ast_connector = to_ast_connector(connector);
> >> - struct ast_private *ast = connector->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(connector->dev);
> >> struct edid *edid;
> >> int ret;
> >> bool flags = false;
> >> @@ -1017,7 +1016,7 @@ static int ast_get_modes(struct drm_connector *connector)
> >> static enum drm_mode_status ast_mode_valid(struct drm_connector *connector,
> >> struct drm_display_mode *mode)
> >> {
> >> - struct ast_private *ast = connector->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(connector->dev);
> >> int flags = MODE_NOMODE;
> >> uint32_t jtemp;
> >>
> >> @@ -1128,7 +1127,7 @@ static int ast_connector_init(struct drm_device *dev)
> >> /* allocate cursor cache and pin at start of VRAM */
> >> static int ast_cursor_init(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> size_t size, i;
> >> struct drm_gem_vram_object *gbo;
> >> int ret;
> >> @@ -1166,7 +1165,7 @@ static int ast_cursor_init(struct drm_device *dev)
> >>
> >> static void ast_cursor_fini(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> size_t i;
> >> struct drm_gem_vram_object *gbo;
> >>
> >> @@ -1179,7 +1178,7 @@ static void ast_cursor_fini(struct drm_device *dev)
> >>
> >> int ast_mode_init(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> int ret;
> >>
> >> memset(&ast->primary_plane, 0, sizeof(ast->primary_plane));
> >> @@ -1223,7 +1222,7 @@ void ast_mode_fini(struct drm_device *dev)
> >> static int get_clock(void *i2c_priv)
> >> {
> >> struct ast_i2c_chan *i2c = i2c_priv;
> >> - struct ast_private *ast = i2c->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(i2c->dev);
> >> uint32_t val, val2, count, pass;
> >>
> >> count = 0;
> >> @@ -1245,7 +1244,7 @@ static int get_clock(void *i2c_priv)
> >> static int get_data(void *i2c_priv)
> >> {
> >> struct ast_i2c_chan *i2c = i2c_priv;
> >> - struct ast_private *ast = i2c->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(i2c->dev);
> >> uint32_t val, val2, count, pass;
> >>
> >> count = 0;
> >> @@ -1267,7 +1266,7 @@ static int get_data(void *i2c_priv)
> >> static void set_clock(void *i2c_priv, int clock)
> >> {
> >> struct ast_i2c_chan *i2c = i2c_priv;
> >> - struct ast_private *ast = i2c->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(i2c->dev);
> >> int i;
> >> u8 ujcrb7, jtemp;
> >>
> >> @@ -1283,7 +1282,7 @@ static void set_clock(void *i2c_priv, int clock)
> >> static void set_data(void *i2c_priv, int data)
> >> {
> >> struct ast_i2c_chan *i2c = i2c_priv;
> >> - struct ast_private *ast = i2c->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(i2c->dev);
> >> int i;
> >> u8 ujcrb7, jtemp;
> >>
> >> @@ -1431,7 +1430,7 @@ static int ast_cursor_move(struct drm_crtc *crtc,
> >> int x, int y)
> >> {
> >> struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> >> - struct ast_private *ast = crtc->dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(crtc->dev);
> >> struct drm_gem_vram_object *gbo;
> >> int x_offset, y_offset;
> >> u8 *dst, *sig;
> >> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> >> index af0c8ebb009a1..226e1290227ad 100644
> >> --- a/drivers/gpu/drm/ast/ast_post.c
> >> +++ b/drivers/gpu/drm/ast/ast_post.c
> >> @@ -39,7 +39,7 @@ static void ast_post_chip_2500(struct drm_device *dev);
> >>
> >> void ast_enable_vga(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >>
> >> ast_io_write8(ast, AST_IO_VGA_ENABLE_PORT, 0x01);
> >> ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
> >> @@ -47,7 +47,7 @@ void ast_enable_vga(struct drm_device *dev)
> >>
> >> void ast_enable_mmio(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >>
> >> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
> >> }
> >> @@ -55,7 +55,7 @@ void ast_enable_mmio(struct drm_device *dev)
> >>
> >> bool ast_is_vga_enabled(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u8 ch;
> >>
> >> ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
> >> @@ -70,7 +70,7 @@ static const u8 extreginfo_ast2300[] = { 0x0f, 0x04, 0x1f, 0xff };
> >> static void
> >> ast_set_def_ext_reg(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u8 i, index, reg;
> >> const u8 *ext_reg_info;
> >>
> >> @@ -272,7 +272,7 @@ static void cbrdlli_ast2150(struct ast_private *ast, int busw)
> >>
> >> static void ast_init_dram_reg(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u8 j;
> >> u32 data, temp, i;
> >> const struct ast_dramstruct *dram_reg_info;
> >> @@ -366,7 +366,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
> >> void ast_post_gpu(struct drm_device *dev)
> >> {
> >> u32 reg;
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >>
> >> pci_read_config_dword(ast->dev->pdev, 0x04, ®);
> >> reg |= 0x3;
> >> @@ -1596,7 +1596,7 @@ static void ddr2_init(struct ast_private *ast, struct ast2300_dram_param *param)
> >>
> >> static void ast_post_chip_2300(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> struct ast2300_dram_param param;
> >> u32 temp;
> >> u8 reg;
> >> @@ -2028,7 +2028,7 @@ static bool ast_dram_init_2500(struct ast_private *ast)
> >>
> >> void ast_post_chip_2500(struct drm_device *dev)
> >> {
> >> - struct ast_private *ast = dev->dev_private;
> >> + struct ast_private *ast = to_ast_private(dev);
> >> u32 temp;
> >> u8 reg;
> >>
> >> --
> >> 2.26.2
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> --
> 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
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] drm/ast: Use per-device logging macros
2020-06-12 6:28 ` Thomas Zimmermann
@ 2020-06-12 7:01 ` Sam Ravnborg
2020-06-12 7:12 ` Thomas Zimmermann
0 siblings, 1 reply; 17+ messages in thread
From: Sam Ravnborg @ 2020-06-12 7:01 UTC (permalink / raw)
To: Thomas Zimmermann; +Cc: airlied, chen, kraxel, dri-devel, emil.velikov
Hi Thomas
On Fri, Jun 12, 2020 at 08:28:40AM +0200, Thomas Zimmermann wrote:
> Hi Sam
>
> Am 11.06.20 um 21:24 schrieb Sam Ravnborg:
> > Hi Thomas.
> > On Thu, Jun 11, 2020 at 10:28:09AM +0200, Thomas Zimmermann wrote:
> >> Converts the ast driver to drm_info() and drm_err(). No functional
> >> changes are made.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> >
> > I hope you will later follow-up with a patch that introduces drm_WARN_*.
>
> I only found DRM_INFO and DRM_ERROR calls. Did I miss any other warning
> macros?
The following:
ast_mode.c: if (WARN_ON_ONCE(gpu_addr < 0))
ast_mode.c: if (WARN_ON_ONCE(fb->width > AST_MAX_HWC_WIDTH) ||
ast_mode.c: WARN_ON_ONCE(fb->height > AST_MAX_HWC_HEIGHT))
ast_mode.c: if (WARN_ON_ONCE(off < 0))
ast_mode.c: if (WARN_ON(!crtc->state))
can benefit from:
/*
* struct drm_device based WARNs
*
* drm_WARN*() acts like WARN*(), but with the key difference of
* using device specific information so that we know from which device
* warning is originating from.
*
* Prefer drm_device based drm_WARN* over regular WARN*
*/
...
#define drm_WARN_ON(drm, x) \
drm_WARN((drm), (x), "%s", \
"drm_WARN_ON(" __stringify(x) ")")
#define drm_WARN_ON_ONCE(drm, x) \
drm_WARN_ONCE((drm), (x), "%s", \
"drm_WARN_ON_ONCE(" __stringify(x) ")")
Also from drm/drm_print.h
Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] drm/ast: Use per-device logging macros
2020-06-12 7:01 ` Sam Ravnborg
@ 2020-06-12 7:12 ` Thomas Zimmermann
0 siblings, 0 replies; 17+ messages in thread
From: Thomas Zimmermann @ 2020-06-12 7:12 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: airlied, chen, kraxel, dri-devel, emil.velikov
[-- Attachment #1.1.1: Type: text/plain, Size: 2237 bytes --]
Hi
Am 12.06.20 um 09:01 schrieb Sam Ravnborg:
> Hi Thomas
>
> On Fri, Jun 12, 2020 at 08:28:40AM +0200, Thomas Zimmermann wrote:
>> Hi Sam
>>
>> Am 11.06.20 um 21:24 schrieb Sam Ravnborg:
>>> Hi Thomas.
>>> On Thu, Jun 11, 2020 at 10:28:09AM +0200, Thomas Zimmermann wrote:
>>>> Converts the ast driver to drm_info() and drm_err(). No functional
>>>> changes are made.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>>>
>>> I hope you will later follow-up with a patch that introduces drm_WARN_*.
>>
>> I only found DRM_INFO and DRM_ERROR calls. Did I miss any other warning
>> macros?
>
> The following:
> ast_mode.c: if (WARN_ON_ONCE(gpu_addr < 0))
> ast_mode.c: if (WARN_ON_ONCE(fb->width > AST_MAX_HWC_WIDTH) ||
> ast_mode.c: WARN_ON_ONCE(fb->height > AST_MAX_HWC_HEIGHT))
> ast_mode.c: if (WARN_ON_ONCE(off < 0))
> ast_mode.c: if (WARN_ON(!crtc->state))
Thanks! I'll fix those.
Best regards
Thomas
>
> can benefit from:
>
> /*
> * struct drm_device based WARNs
> *
> * drm_WARN*() acts like WARN*(), but with the key difference of
> * using device specific information so that we know from which device
> * warning is originating from.
> *
> * Prefer drm_device based drm_WARN* over regular WARN*
> */
>
> ...
>
> #define drm_WARN_ON(drm, x) \
> drm_WARN((drm), (x), "%s", \
> "drm_WARN_ON(" __stringify(x) ")")
>
> #define drm_WARN_ON_ONCE(drm, x) \
> drm_WARN_ONCE((drm), (x), "%s", \
> "drm_WARN_ON_ONCE(" __stringify(x) ")")
>
>
> Also from drm/drm_print.h
>
> Sam
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180
2020-06-11 8:28 ` [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180 Thomas Zimmermann
2020-06-11 17:24 ` Daniel Vetter
@ 2020-06-15 23:21 ` Emil Velikov
2020-06-17 7:13 ` Thomas Zimmermann
1 sibling, 1 reply; 17+ messages in thread
From: Emil Velikov @ 2020-06-15 23:21 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: chen, ML dri-devel, Gerd Hoffmann, Dave Airlie, Sam Ravnborg,
Emil Velikov
Hi Thomas,
On Thu, 11 Jun 2020 at 09:28, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -59,7 +59,6 @@ static struct drm_driver driver;
> static const struct pci_device_id pciidlist[] = {
> AST_VGA_DEVICE(PCI_CHIP_AST2000, NULL),
> AST_VGA_DEVICE(PCI_CHIP_AST2100, NULL),
> - /* AST_VGA_DEVICE(PCI_CHIP_AST1180, NULL), - don't bind to 1180 for now */
Since we don't bind to this pciid, the (removed) code is never
used/dead. For the series:
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Small idea below: feel free to ignore or if you agree - follow-up at a
random point in the future.
> + if (dev->pdev->revision >= 0x40) {
> + ast->chip = AST2500;
> + DRM_INFO("AST 2500 detected\n");
> + } else if (dev->pdev->revision >= 0x30) {
> + ast->chip = AST2400;
> + DRM_INFO("AST 2400 detected\n");
> + } else if (dev->pdev->revision >= 0x30) {
> + ast->chip = AST2400;
> + DRM_INFO("AST 2400 detected\n");
> + } else if (dev->pdev->revision >= 0x20) {
> + ast->chip = AST2300;
> + DRM_INFO("AST 2300 detected\n");
> + } else if (dev->pdev->revision >= 0x10) {
> + switch (scu_rev & 0x0300) {
> + case 0x0200:
> + ast->chip = AST1100;
> + DRM_INFO("AST 1100 detected\n");
> + break;
> + case 0x0100:
> + ast->chip = AST2200;
> + DRM_INFO("AST 2200 detected\n");
> + break;
> + case 0x0000:
> + ast->chip = AST2150;
> + DRM_INFO("AST 2150 detected\n");
> + break;
> + default:
> + ast->chip = AST2100;
> + DRM_INFO("AST 2100 detected\n");
> + break;
> }
> + ast->vga2_clone = false;
> + } else {
> + ast->chip = AST2000;
> + DRM_INFO("AST 2000 detected\n");
> }
>
Instead of the above if/else spaghetti, one can use something alike
static const struct foo {
u8 rev_maj; // revision & 0xf0 >> 4
u8 rev_scu; // scu_rev & 0x0300 >> 8, ignored if table has 0xf
enum ast_chip;
const char *name;
} bar {
{ 0x3, 0xf, AST2400, "2400" },
{ 0x2, 0xf, AST2300, "2300" },
{ 0x1, 0x3, AST2100, "2100" },
{ 0x1, 0x2, AST1100, "1100" },
{ 0x1, 0x1, AST2200, "2200" },
{ 0x1, 0x0, AST2150, "2150" },
{ 0x0, 0xf, AST2000, "2000" },
};
+ trivial loop.
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180
2020-06-15 23:21 ` Emil Velikov
@ 2020-06-17 7:13 ` Thomas Zimmermann
0 siblings, 0 replies; 17+ messages in thread
From: Thomas Zimmermann @ 2020-06-17 7:13 UTC (permalink / raw)
To: Emil Velikov
Cc: chen, ML dri-devel, Gerd Hoffmann, Dave Airlie, Sam Ravnborg,
Emil Velikov
[-- Attachment #1.1.1: Type: text/plain, Size: 3596 bytes --]
Hi Emil
Am 16.06.20 um 01:21 schrieb Emil Velikov:
> Hi Thomas,
>
> On Thu, 11 Jun 2020 at 09:28, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
>> --- a/drivers/gpu/drm/ast/ast_drv.c
>> +++ b/drivers/gpu/drm/ast/ast_drv.c
>> @@ -59,7 +59,6 @@ static struct drm_driver driver;
>> static const struct pci_device_id pciidlist[] = {
>> AST_VGA_DEVICE(PCI_CHIP_AST2000, NULL),
>> AST_VGA_DEVICE(PCI_CHIP_AST2100, NULL),
>> - /* AST_VGA_DEVICE(PCI_CHIP_AST1180, NULL), - don't bind to 1180 for now */
>
> Since we don't bind to this pciid, the (removed) code is never
> used/dead. For the series:
> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
>
> Small idea below: feel free to ignore or if you agree - follow-up at a
> random point in the future.
>
>
>> + if (dev->pdev->revision >= 0x40) {
>> + ast->chip = AST2500;
>> + DRM_INFO("AST 2500 detected\n");
>> + } else if (dev->pdev->revision >= 0x30) {
>> + ast->chip = AST2400;
>> + DRM_INFO("AST 2400 detected\n");
>> + } else if (dev->pdev->revision >= 0x30) {
>> + ast->chip = AST2400;
>> + DRM_INFO("AST 2400 detected\n");
>> + } else if (dev->pdev->revision >= 0x20) {
>> + ast->chip = AST2300;
>> + DRM_INFO("AST 2300 detected\n");
>> + } else if (dev->pdev->revision >= 0x10) {
>> + switch (scu_rev & 0x0300) {
>> + case 0x0200:
>> + ast->chip = AST1100;
>> + DRM_INFO("AST 1100 detected\n");
>> + break;
>> + case 0x0100:
>> + ast->chip = AST2200;
>> + DRM_INFO("AST 2200 detected\n");
>> + break;
>> + case 0x0000:
>> + ast->chip = AST2150;
>> + DRM_INFO("AST 2150 detected\n");
>> + break;
>> + default:
>> + ast->chip = AST2100;
>> + DRM_INFO("AST 2100 detected\n");
>> + break;
>> }
>> + ast->vga2_clone = false;
>> + } else {
>> + ast->chip = AST2000;
>> + DRM_INFO("AST 2000 detected\n");
>> }
>>
> Instead of the above if/else spaghetti, one can use something alike
>
> static const struct foo {
> u8 rev_maj; // revision & 0xf0 >> 4
> u8 rev_scu; // scu_rev & 0x0300 >> 8, ignored if table has 0xf
> enum ast_chip;
> const char *name;
> } bar {
> { 0x3, 0xf, AST2400, "2400" },
> { 0x2, 0xf, AST2300, "2300" },
> { 0x1, 0x3, AST2100, "2100" },
> { 0x1, 0x2, AST1100, "1100" },
> { 0x1, 0x1, AST2200, "2200" },
> { 0x1, 0x0, AST2150, "2150" },
> { 0x0, 0xf, AST2000, "2000" },
> };
>
> + trivial loop.
I do like the idea, but there's plenty of similar code throughout the
driver. I think a separate patchset could introduce an info structure
with per-chip constants and flags, and fix the entire driver.
Best regards
Thomas
>
> -Emil
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-06-17 7:13 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 8:28 [PATCH 0/3] ast cleanups Thomas Zimmermann
2020-06-11 8:28 ` [PATCH 1/3] drm/ast: Remove unused code paths for AST 1180 Thomas Zimmermann
2020-06-11 17:24 ` Daniel Vetter
2020-06-15 23:21 ` Emil Velikov
2020-06-17 7:13 ` Thomas Zimmermann
2020-06-11 8:28 ` [PATCH 2/3] drm/ast: Upcast from DRM device to ast structure via to_ast_private() Thomas Zimmermann
2020-06-11 17:32 ` Daniel Vetter
2020-06-12 6:31 ` Thomas Zimmermann
2020-06-11 19:23 ` Sam Ravnborg
2020-06-12 6:30 ` Thomas Zimmermann
2020-06-12 6:57 ` Daniel Vetter
2020-06-11 8:28 ` [PATCH 3/3] drm/ast: Use per-device logging macros Thomas Zimmermann
2020-06-11 17:33 ` Daniel Vetter
2020-06-11 19:24 ` Sam Ravnborg
2020-06-12 6:28 ` Thomas Zimmermann
2020-06-12 7:01 ` Sam Ravnborg
2020-06-12 7:12 ` Thomas Zimmermann
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.