* [PATCH 04/11] drm: cirrus: remove dead code and pointless local lut storage
From: Peter Rosin @ 2017-06-20 19:25 UTC (permalink / raw)
To: linux-kernel
Cc: Patrik Jakobsson, Boris Brezillon, amd-gfx, intel-gfx,
David Airlie, nouveau, dri-devel, Philippe Cornu, Jani Nikula,
Christian König, Yannick Fertre, Sean Paul,
Benjamin Gaignard, Daniel Vetter, Alex Deucher, Dave Airlie,
virtualization, Vincent Abriou, Peter Rosin, Ben Skeggs
In-Reply-To: <1497986735-14418-1-git-send-email-peda@axentia.se>
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/cirrus/cirrus_drv.h | 8 ----
drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 -
drivers/gpu/drm/cirrus/cirrus_mode.c | 71 ++++++++---------------------------
3 files changed, 16 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 8690352..be2d7e48 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -96,7 +96,6 @@
struct cirrus_crtc {
struct drm_crtc base;
- u8 lut_r[256], lut_g[256], lut_b[256];
int last_dpms;
bool enabled;
};
@@ -180,13 +179,6 @@ cirrus_bo(struct ttm_buffer_object *bo)
#define to_cirrus_obj(x) container_of(x, struct cirrus_gem_object, base)
#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
- /* cirrus_mode.c */
-void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno);
-void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno);
-
-
/* cirrus_main.c */
int cirrus_device_init(struct cirrus_device *cdev,
struct drm_device *ddev,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 7fa58ee..1fedab0 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -265,8 +265,6 @@ static int cirrus_fbdev_destroy(struct drm_device *dev,
}
static const struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
- .gamma_set = cirrus_crtc_fb_gamma_set,
- .gamma_get = cirrus_crtc_fb_gamma_get,
.fb_probe = cirrusfb_create,
};
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 53f6f0f..a4c4a46 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -31,25 +31,6 @@
* This file contains setup code for the CRTC.
*/
-static void cirrus_crtc_load_lut(struct drm_crtc *crtc)
-{
- struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
- struct drm_device *dev = crtc->dev;
- struct cirrus_device *cdev = dev->dev_private;
- int i;
-
- if (!crtc->enabled)
- return;
-
- for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
- /* VGA registers */
- WREG8(PALETTE_INDEX, i);
- WREG8(PALETTE_DATA, cirrus_crtc->lut_r[i]);
- WREG8(PALETTE_DATA, cirrus_crtc->lut_g[i]);
- WREG8(PALETTE_DATA, cirrus_crtc->lut_b[i]);
- }
-}
-
/*
* The DRM core requires DPMS functions, but they make little sense in our
* case and so are just stubs
@@ -330,15 +311,25 @@ static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
+ struct drm_device *dev = crtc->dev;
+ struct cirrus_device *cdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
- for (i = 0; i < size; i++) {
- cirrus_crtc->lut_r[i] = red[i];
- cirrus_crtc->lut_g[i] = green[i];
- cirrus_crtc->lut_b[i] = blue[i];
+ if (!crtc->enabled)
+ return 0;
+
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
+ for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
+ /* VGA registers */
+ WREG8(PALETTE_INDEX, i);
+ WREG8(PALETTE_DATA, *r++ >> 8);
+ WREG8(PALETTE_DATA, *g++ >> 8);
+ WREG8(PALETTE_DATA, *b++ >> 8);
}
- cirrus_crtc_load_lut(crtc);
return 0;
}
@@ -365,7 +356,6 @@ static const struct drm_crtc_helper_funcs cirrus_helper_funcs = {
.mode_set_base = cirrus_crtc_mode_set_base,
.prepare = cirrus_crtc_prepare,
.commit = cirrus_crtc_commit,
- .load_lut = cirrus_crtc_load_lut,
};
/* CRTC setup */
@@ -373,7 +363,6 @@ static void cirrus_crtc_init(struct drm_device *dev)
{
struct cirrus_device *cdev = dev->dev_private;
struct cirrus_crtc *cirrus_crtc;
- int i;
cirrus_crtc = kzalloc(sizeof(struct cirrus_crtc) +
(CIRRUSFB_CONN_LIMIT * sizeof(struct drm_connector *)),
@@ -387,37 +376,9 @@ static void cirrus_crtc_init(struct drm_device *dev)
drm_mode_crtc_set_gamma_size(&cirrus_crtc->base, CIRRUS_LUT_SIZE);
cdev->mode_info.crtc = cirrus_crtc;
- for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
- cirrus_crtc->lut_r[i] = i;
- cirrus_crtc->lut_g[i] = i;
- cirrus_crtc->lut_b[i] = i;
- }
-
drm_crtc_helper_add(&cirrus_crtc->base, &cirrus_helper_funcs);
}
-/** Sets the color ramps on behalf of fbcon */
-void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-
- cirrus_crtc->lut_r[regno] = red;
- cirrus_crtc->lut_g[regno] = green;
- cirrus_crtc->lut_b[regno] = blue;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno)
-{
- struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
-
- *red = cirrus_crtc->lut_r[regno];
- *green = cirrus_crtc->lut_g[regno];
- *blue = cirrus_crtc->lut_b[regno];
-}
-
static void cirrus_encoder_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
--
2.1.4
^ permalink raw reply related
* [PATCH 05/11] dmr: gma500: remove dead code and pointless local lut storage
From: Peter Rosin @ 2017-06-20 19:25 UTC (permalink / raw)
To: linux-kernel
Cc: Patrik Jakobsson, Boris Brezillon, amd-gfx, intel-gfx,
David Airlie, nouveau, dri-devel, Philippe Cornu, Jani Nikula,
Christian König, Yannick Fertre, Sean Paul,
Benjamin Gaignard, Daniel Vetter, Alex Deucher, Dave Airlie,
virtualization, Vincent Abriou, Peter Rosin, Ben Skeggs
In-Reply-To: <1497986735-14418-1-git-send-email-peda@axentia.se>
The redundant fb helpers .gamma_set and .gamma_get are no longer
used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
It is a bit strange that the fb helper .load_lut was not hooked
up, so this change may well make the driver work for the C8 mode
from the fbdev interface. But that is untested.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/gma500/framebuffer.c | 22 --------------------
drivers/gpu/drm/gma500/gma_display.c | 32 ++++++++++--------------------
drivers/gpu/drm/gma500/psb_intel_display.c | 7 +------
drivers/gpu/drm/gma500/psb_intel_drv.h | 1 -
4 files changed, 12 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c
index 7da70b6..2570c7f 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -479,26 +479,6 @@ static struct drm_framebuffer *psb_user_framebuffer_create
return psb_framebuffer_create(dev, cmd, r);
}
-static void psbfb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-
- gma_crtc->lut_r[regno] = red >> 8;
- gma_crtc->lut_g[regno] = green >> 8;
- gma_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void psbfb_gamma_get(struct drm_crtc *crtc, u16 *red,
- u16 *green, u16 *blue, int regno)
-{
- struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-
- *red = gma_crtc->lut_r[regno] << 8;
- *green = gma_crtc->lut_g[regno] << 8;
- *blue = gma_crtc->lut_b[regno] << 8;
-}
-
static int psbfb_probe(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes)
{
@@ -525,8 +505,6 @@ static int psbfb_probe(struct drm_fb_helper *helper,
}
static const struct drm_fb_helper_funcs psb_fb_helper_funcs = {
- .gamma_set = psbfb_gamma_set,
- .gamma_get = psbfb_gamma_get,
.fb_probe = psbfb_probe,
};
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index e7fd356..f3c48a2 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -144,33 +144,32 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
const struct psb_offset *map = &dev_priv->regmap[gma_crtc->pipe];
int palreg = map->palette;
+ u16 *r, *g, *b;
int i;
/* The clocks have to be on to load the palette. */
if (!crtc->enabled)
return;
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
if (gma_power_begin(dev, false)) {
for (i = 0; i < 256; i++) {
REG_WRITE(palreg + 4 * i,
- ((gma_crtc->lut_r[i] +
- gma_crtc->lut_adj[i]) << 16) |
- ((gma_crtc->lut_g[i] +
- gma_crtc->lut_adj[i]) << 8) |
- (gma_crtc->lut_b[i] +
- gma_crtc->lut_adj[i]));
+ (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+ (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+ ((*b++ >> 8) + gma_crtc->lut_adj[i]));
}
gma_power_end(dev);
} else {
for (i = 0; i < 256; i++) {
/* FIXME: Why pipe[0] and not pipe[..._crtc->pipe]? */
dev_priv->regs.pipe[0].palette[i] =
- ((gma_crtc->lut_r[i] +
- gma_crtc->lut_adj[i]) << 16) |
- ((gma_crtc->lut_g[i] +
- gma_crtc->lut_adj[i]) << 8) |
- (gma_crtc->lut_b[i] +
- gma_crtc->lut_adj[i]);
+ (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+ (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+ ((*b++ >> 8) + gma_crtc->lut_adj[i]);
}
}
@@ -180,15 +179,6 @@ int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
u32 size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
- int i;
-
- for (i = 0; i < size; i++) {
- gma_crtc->lut_r[i] = red[i] >> 8;
- gma_crtc->lut_g[i] = green[i] >> 8;
- gma_crtc->lut_b[i] = blue[i] >> 8;
- }
-
gma_crtc_load_lut(crtc);
return 0;
diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c
index 7b6c849..8762efa 100644
--- a/drivers/gpu/drm/gma500/psb_intel_display.c
+++ b/drivers/gpu/drm/gma500/psb_intel_display.c
@@ -518,13 +518,8 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe,
gma_crtc->pipe = pipe;
gma_crtc->plane = pipe;
- for (i = 0; i < 256; i++) {
- gma_crtc->lut_r[i] = i;
- gma_crtc->lut_g[i] = i;
- gma_crtc->lut_b[i] = i;
-
+ for (i = 0; i < 256; i++)
gma_crtc->lut_adj[i] = 0;
- }
gma_crtc->mode_dev = mode_dev;
gma_crtc->cursor_addr = 0;
diff --git a/drivers/gpu/drm/gma500/psb_intel_drv.h b/drivers/gpu/drm/gma500/psb_intel_drv.h
index 6a10215..e8e4ea1 100644
--- a/drivers/gpu/drm/gma500/psb_intel_drv.h
+++ b/drivers/gpu/drm/gma500/psb_intel_drv.h
@@ -172,7 +172,6 @@ struct gma_crtc {
int plane;
uint32_t cursor_addr;
struct gtt_range *cursor_gt;
- u8 lut_r[256], lut_g[256], lut_b[256];
u8 lut_adj[256];
struct psb_intel_framebuffer *fbdev_fb;
/* a mode_set for fbdev users on this crtc */
--
2.1.4
^ permalink raw reply related
* [PATCH 06/11] drm: i915: remove dead code and pointless local lut storage
From: Peter Rosin @ 2017-06-20 19:25 UTC (permalink / raw)
To: linux-kernel
Cc: Patrik Jakobsson, Boris Brezillon, amd-gfx, intel-gfx,
David Airlie, nouveau, dri-devel, Philippe Cornu, Jani Nikula,
Christian König, Yannick Fertre, Sean Paul,
Benjamin Gaignard, Daniel Vetter, Alex Deucher, Dave Airlie,
virtualization, Vincent Abriou, Peter Rosin, Ben Skeggs
In-Reply-To: <1497986735-14418-1-git-send-email-peda@axentia.se>
The driver stores lut values from the fbdev interface, and is able
to give them back, but does not appear to do anything with these
lut values. The generic fb helpers have replaced this function,
and may even have made the driver work for the C8 mode from the
fbdev interface. But that is untested.
Since the fb helper .gamma_set and .gamma_get are no longer used,
just kill the mysterious dead code.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_fbdev.c | 31 -------------------------------
2 files changed, 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 83dd409..503edf3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -785,7 +785,6 @@ struct intel_crtc {
struct drm_crtc base;
enum pipe pipe;
enum plane plane;
- u8 lut_r[256], lut_g[256], lut_b[256];
/*
* Whether the crtc and the connected output pipeline is active. Implies
* that crtc->enabled is set, i.e. the current mode configuration has
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 03347c6..5bac953 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -281,27 +281,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
return ret;
}
-/** Sets the color ramps on behalf of RandR */
-static void intel_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- intel_crtc->lut_r[regno] = red >> 8;
- intel_crtc->lut_g[regno] = green >> 8;
- intel_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- *red = intel_crtc->lut_r[regno] << 8;
- *green = intel_crtc->lut_g[regno] << 8;
- *blue = intel_crtc->lut_b[regno] << 8;
-}
-
static struct drm_fb_helper_crtc *
intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
{
@@ -370,7 +349,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
struct drm_connector *connector;
struct drm_encoder *encoder;
struct drm_fb_helper_crtc *new_crtc;
- struct intel_crtc *intel_crtc;
fb_conn = fb_helper->connector_info[i];
connector = fb_conn->connector;
@@ -412,13 +390,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
num_connectors_enabled++;
- intel_crtc = to_intel_crtc(connector->state->crtc);
- for (j = 0; j < 256; j++) {
- intel_crtc->lut_r[j] = j;
- intel_crtc->lut_g[j] = j;
- intel_crtc->lut_b[j] = j;
- }
-
new_crtc = intel_fb_helper_crtc(fb_helper,
connector->state->crtc);
@@ -519,8 +490,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
.initial_config = intel_fb_initial_config,
- .gamma_set = intel_crtc_fb_gamma_set,
- .gamma_get = intel_crtc_fb_gamma_get,
.fb_probe = intelfb_create,
};
--
2.1.4
^ permalink raw reply related
* [PATCH 07/11] drm: mgag200: remove dead code and pointless local lut storage
From: Peter Rosin @ 2017-06-20 19:25 UTC (permalink / raw)
To: linux-kernel
Cc: Patrik Jakobsson, Boris Brezillon, amd-gfx, intel-gfx,
David Airlie, nouveau, dri-devel, Philippe Cornu, Jani Nikula,
Christian König, Yannick Fertre, Sean Paul,
Benjamin Gaignard, Daniel Vetter, Alex Deucher, Dave Airlie,
virtualization, Vincent Abriou, Peter Rosin, Ben Skeggs
In-Reply-To: <1497986735-14418-1-git-send-email-peda@axentia.se>
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/mgag200/mgag200_drv.h | 5 ---
drivers/gpu/drm/mgag200/mgag200_fb.c | 2 --
drivers/gpu/drm/mgag200/mgag200_mode.c | 62 ++++++++--------------------------
3 files changed, 15 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index c88b6ec..04f1dfb 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -237,11 +237,6 @@ mgag200_bo(struct ttm_buffer_object *bo)
{
return container_of(bo, struct mgag200_bo, bo);
}
- /* mgag200_crtc.c */
-void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno);
-void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno);
/* mgag200_mode.c */
int mgag200_modeset_init(struct mga_device *mdev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 5d3b1fa..5cf980a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -258,8 +258,6 @@ static int mga_fbdev_destroy(struct drm_device *dev,
}
static const struct drm_fb_helper_funcs mga_fb_helper_funcs = {
- .gamma_set = mga_crtc_fb_gamma_set,
- .gamma_get = mga_crtc_fb_gamma_get,
.fb_probe = mgag200fb_create,
};
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index adb411a..117bec3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -27,15 +27,19 @@
static void mga_crtc_load_lut(struct drm_crtc *crtc)
{
- struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct mga_device *mdev = dev->dev_private;
struct drm_framebuffer *fb = crtc->primary->fb;
+ u16 *r_ptr, *g_ptr, *b_ptr;
int i;
if (!crtc->enabled)
return;
+ r_ptr = crtc->gamma_store;
+ g_ptr = r_ptr + crtc->gamma_size;
+ b_ptr = g_ptr + crtc->gamma_size;
+
WREG8(DAC_INDEX + MGA1064_INDEX, 0);
if (fb && fb->format->cpp[0] * 8 == 16) {
@@ -46,25 +50,27 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
if (i > (MGAG200_LUT_SIZE >> 1)) {
r = b = 0;
} else {
- r = mga_crtc->lut_r[i << 1];
- b = mga_crtc->lut_b[i << 1];
+ r = *r_ptr++ >> 8;
+ b = *b_ptr++ >> 8;
+ r_ptr++;
+ b_ptr++;
}
} else {
- r = mga_crtc->lut_r[i];
- b = mga_crtc->lut_b[i];
+ r = *r_ptr++ >> 8;
+ b = *b_ptr++ >> 8;
}
/* VGA registers */
WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
}
return;
}
for (i = 0; i < MGAG200_LUT_SIZE; i++) {
/* VGA registers */
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_r[i]);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_b[i]);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *r_ptr++ >> 8);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *b_ptr++ >> 8);
}
}
@@ -1396,14 +1402,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
- int i;
-
- for (i = 0; i < size; i++) {
- mga_crtc->lut_r[i] = red[i] >> 8;
- mga_crtc->lut_g[i] = green[i] >> 8;
- mga_crtc->lut_b[i] = blue[i] >> 8;
- }
mga_crtc_load_lut(crtc);
return 0;
@@ -1452,14 +1450,12 @@ static const struct drm_crtc_helper_funcs mga_helper_funcs = {
.mode_set_base = mga_crtc_mode_set_base,
.prepare = mga_crtc_prepare,
.commit = mga_crtc_commit,
- .load_lut = mga_crtc_load_lut,
};
/* CRTC setup */
static void mga_crtc_init(struct mga_device *mdev)
{
struct mga_crtc *mga_crtc;
- int i;
mga_crtc = kzalloc(sizeof(struct mga_crtc) +
(MGAG200FB_CONN_LIMIT * sizeof(struct drm_connector *)),
@@ -1473,37 +1469,9 @@ static void mga_crtc_init(struct mga_device *mdev)
drm_mode_crtc_set_gamma_size(&mga_crtc->base, MGAG200_LUT_SIZE);
mdev->mode_info.crtc = mga_crtc;
- for (i = 0; i < MGAG200_LUT_SIZE; i++) {
- mga_crtc->lut_r[i] = i;
- mga_crtc->lut_g[i] = i;
- mga_crtc->lut_b[i] = i;
- }
-
drm_crtc_helper_add(&mga_crtc->base, &mga_helper_funcs);
}
-/** Sets the color ramps on behalf of fbcon */
-void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-
- mga_crtc->lut_r[regno] = red >> 8;
- mga_crtc->lut_g[regno] = green >> 8;
- mga_crtc->lut_b[regno] = blue >> 8;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno)
-{
- struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-
- *red = (u16)mga_crtc->lut_r[regno] << 8;
- *green = (u16)mga_crtc->lut_g[regno] << 8;
- *blue = (u16)mga_crtc->lut_b[regno] << 8;
-}
-
/*
* The encoder comes after the CRTC in the output pipeline, but before
* the connector. It's responsible for ensuring that the digital
--
2.1.4
^ permalink raw reply related
* [PATCH 08/11] drm: nouveau: remove dead code and pointless local lut storage
From: Peter Rosin @ 2017-06-20 19:25 UTC (permalink / raw)
To: linux-kernel
Cc: Patrik Jakobsson, Boris Brezillon, amd-gfx, intel-gfx,
David Airlie, nouveau, dri-devel, Philippe Cornu, Jani Nikula,
Christian König, Yannick Fertre, Sean Paul,
Benjamin Gaignard, Daniel Vetter, Alex Deucher, Dave Airlie,
virtualization, Vincent Abriou, Peter Rosin, Ben Skeggs
In-Reply-To: <1497986735-14418-1-git-send-email-peda@axentia.se>
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 26 ++++++++--------------
drivers/gpu/drm/nouveau/nouveau_crtc.h | 3 ---
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 22 -------------------
drivers/gpu/drm/nouveau/nv50_display.c | 39 ++++++++++-----------------------
4 files changed, 21 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 4b4b0b4..8f689f1 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -764,13 +764,18 @@ nv_crtc_gamma_load(struct drm_crtc *crtc)
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
struct drm_device *dev = nv_crtc->base.dev;
struct rgb { uint8_t r, g, b; } __attribute__((packed)) *rgbs;
+ u16 *r, *g, *b;
int i;
rgbs = (struct rgb *)nv04_display(dev)->mode_reg.crtc_reg[nv_crtc->index].DAC;
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
for (i = 0; i < 256; i++) {
- rgbs[i].r = nv_crtc->lut.r[i] >> 8;
- rgbs[i].g = nv_crtc->lut.g[i] >> 8;
- rgbs[i].b = nv_crtc->lut.b[i] >> 8;
+ rgbs[i].r = *r++ >> 8;
+ rgbs[i].g = *g++ >> 8;
+ rgbs[i].b = *b++ >> 8;
}
nouveau_hw_load_state_palette(dev, nv_crtc->index, &nv04_display(dev)->mode_reg);
@@ -792,13 +797,6 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
struct drm_modeset_acquire_ctx *ctx)
{
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
- int i;
-
- for (i = 0; i < size; i++) {
- nv_crtc->lut.r[i] = r[i];
- nv_crtc->lut.g[i] = g[i];
- nv_crtc->lut.b[i] = b[i];
- }
/* We need to know the depth before we upload, but it's possible to
* get called before a framebuffer is bound. If this is the case,
@@ -1095,7 +1093,6 @@ static const struct drm_crtc_helper_funcs nv04_crtc_helper_funcs = {
.mode_set = nv_crtc_mode_set,
.mode_set_base = nv04_crtc_mode_set_base,
.mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
- .load_lut = nv_crtc_gamma_load,
.disable = nv_crtc_disable,
};
@@ -1103,17 +1100,12 @@ int
nv04_crtc_create(struct drm_device *dev, int crtc_num)
{
struct nouveau_crtc *nv_crtc;
- int ret, i;
+ int ret;
nv_crtc = kzalloc(sizeof(*nv_crtc), GFP_KERNEL);
if (!nv_crtc)
return -ENOMEM;
- for (i = 0; i < 256; i++) {
- nv_crtc->lut.r[i] = i << 8;
- nv_crtc->lut.g[i] = i << 8;
- nv_crtc->lut.b[i] = i << 8;
- }
nv_crtc->lut.depth = 0;
nv_crtc->index = crtc_num;
diff --git a/drivers/gpu/drm/nouveau/nouveau_crtc.h b/drivers/gpu/drm/nouveau/nouveau_crtc.h
index 050fcf3..b7a18fb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_crtc.h
+++ b/drivers/gpu/drm/nouveau/nouveau_crtc.h
@@ -61,9 +61,6 @@ struct nouveau_crtc {
struct {
struct nouveau_bo *nvbo;
- uint16_t r[256];
- uint16_t g[256];
- uint16_t b[256];
int depth;
} lut;
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 2665a07..f770784 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -278,26 +278,6 @@ nouveau_fbcon_accel_init(struct drm_device *dev)
info->fbops = &nouveau_fbcon_ops;
}
-static void nouveau_fbcon_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
- nv_crtc->lut.r[regno] = red;
- nv_crtc->lut.g[regno] = green;
- nv_crtc->lut.b[regno] = blue;
-}
-
-static void nouveau_fbcon_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno)
-{
- struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
- *red = nv_crtc->lut.r[regno];
- *green = nv_crtc->lut.g[regno];
- *blue = nv_crtc->lut.b[regno];
-}
-
static void
nouveau_fbcon_zfill(struct drm_device *dev, struct nouveau_fbdev *fbcon)
{
@@ -467,8 +447,6 @@ void nouveau_fbcon_gpu_lockup(struct fb_info *info)
}
static const struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
- .gamma_set = nouveau_fbcon_gamma_set,
- .gamma_get = nouveau_fbcon_gamma_get,
.fb_probe = nouveau_fbcon_create,
};
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 775c100..3686f5b 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2193,28 +2193,28 @@ nv50_head_lut_load(struct drm_crtc *crtc)
struct nv50_disp *disp = nv50_disp(crtc->dev);
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
void __iomem *lut = nvbo_kmap_obj_iovirtual(nv_crtc->lut.nvbo);
+ u16 *r, *g, *b;
int i;
- for (i = 0; i < 256; i++) {
- u16 r = nv_crtc->lut.r[i] >> 2;
- u16 g = nv_crtc->lut.g[i] >> 2;
- u16 b = nv_crtc->lut.b[i] >> 2;
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+ for (i = 0; i < 256; i++) {
if (disp->disp->oclass < GF110_DISP) {
- writew(r + 0x0000, lut + (i * 0x08) + 0);
- writew(g + 0x0000, lut + (i * 0x08) + 2);
- writew(b + 0x0000, lut + (i * 0x08) + 4);
+ writew((*r++ >> 2) + 0x0000, lut + (i * 0x08) + 0);
+ writew((*g++ >> 2) + 0x0000, lut + (i * 0x08) + 2);
+ writew((*b++ >> 2) + 0x0000, lut + (i * 0x08) + 4);
} else {
- writew(r + 0x6000, lut + (i * 0x20) + 0);
- writew(g + 0x6000, lut + (i * 0x20) + 2);
- writew(b + 0x6000, lut + (i * 0x20) + 4);
+ writew((*r++ >> 2) + 0x6000, lut + (i * 0x20) + 0);
+ writew((*g++ >> 2) + 0x6000, lut + (i * 0x20) + 2);
+ writew((*b++ >> 2) + 0x6000, lut + (i * 0x20) + 4);
}
}
}
static const struct drm_crtc_helper_funcs
nv50_head_help = {
- .load_lut = nv50_head_lut_load,
.atomic_check = nv50_head_atomic_check,
};
@@ -2223,15 +2223,6 @@ nv50_head_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
- u32 i;
-
- for (i = 0; i < size; i++) {
- nv_crtc->lut.r[i] = r[i];
- nv_crtc->lut.g[i] = g[i];
- nv_crtc->lut.b[i] = b[i];
- }
-
nv50_head_lut_load(crtc);
return 0;
}
@@ -2329,19 +2320,13 @@ nv50_head_create(struct drm_device *dev, int index)
struct nv50_base *base;
struct nv50_curs *curs;
struct drm_crtc *crtc;
- int ret, i;
+ int ret;
head = kzalloc(sizeof(*head), GFP_KERNEL);
if (!head)
return -ENOMEM;
head->base.index = index;
- for (i = 0; i < 256; i++) {
- head->base.lut.r[i] = i << 8;
- head->base.lut.g[i] = i << 8;
- head->base.lut.b[i] = i << 8;
- }
-
ret = nv50_base_new(drm, head, &base);
if (ret == 0)
ret = nv50_curs_new(drm, head, &curs);
--
2.1.4
^ permalink raw reply related
* [PATCH 09/11] drm: radeon: remove dead code and pointless local lut storage
From: Peter Rosin @ 2017-06-20 19:25 UTC (permalink / raw)
To: linux-kernel
Cc: Patrik Jakobsson, Boris Brezillon, amd-gfx, intel-gfx,
David Airlie, nouveau, dri-devel, Philippe Cornu, Jani Nikula,
Christian König, Yannick Fertre, Sean Paul,
Benjamin Gaignard, Daniel Vetter, Alex Deucher, Dave Airlie,
virtualization, Vincent Abriou, Peter Rosin, Ben Skeggs
In-Reply-To: <1497986735-14418-1-git-send-email-peda@axentia.se>
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/radeon/atombios_crtc.c | 1 -
drivers/gpu/drm/radeon/radeon_connectors.c | 7 ++-
drivers/gpu/drm/radeon/radeon_display.c | 71 ++++++++++++-----------------
drivers/gpu/drm/radeon/radeon_fb.c | 2 -
drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 1 -
5 files changed, 33 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 3c492a0..02baaaf 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -2217,7 +2217,6 @@ static const struct drm_crtc_helper_funcs atombios_helper_funcs = {
.mode_set_base_atomic = atombios_crtc_set_base_atomic,
.prepare = atombios_crtc_prepare,
.commit = atombios_crtc_commit,
- .load_lut = radeon_crtc_load_lut,
.disable = atombios_crtc_disable,
};
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 27affbd..2f642cb 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -773,12 +773,15 @@ static int radeon_connector_set_property(struct drm_connector *connector, struct
if (connector->encoder->crtc) {
struct drm_crtc *crtc = connector->encoder->crtc;
- const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
radeon_crtc->output_csc = radeon_encoder->output_csc;
- (*crtc_funcs->load_lut)(crtc);
+ /*
+ * Our .gamma_set assumes the .gamma_store has been
+ * prefilled and don't care about its arguments.
+ */
+ crtc->funcs->gamma_set(crtc, NULL, NULL, NULL, 0, NULL);
}
}
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 17d3daf..8b7d7a0 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -42,6 +42,7 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -60,11 +61,14 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
WREG32(AVIVO_DC_LUT_WRITE_EN_MASK, 0x0000003f);
WREG8(AVIVO_DC_LUT_RW_INDEX, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(AVIVO_DC_LUT_30_COLOR,
- (radeon_crtc->lut_r[i] << 20) |
- (radeon_crtc->lut_g[i] << 10) |
- (radeon_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
/* Only change bit 0 of LUT_SEL, other bits are set elsewhere */
@@ -76,6 +80,7 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -93,11 +98,14 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
- (radeon_crtc->lut_r[i] << 20) |
- (radeon_crtc->lut_g[i] << 10) |
- (radeon_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
}
@@ -106,6 +114,7 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -135,11 +144,14 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset, 0x00000007);
WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
- (radeon_crtc->lut_r[i] << 20) |
- (radeon_crtc->lut_g[i] << 10) |
- (radeon_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
WREG32(NI_DEGAMMA_CONTROL + radeon_crtc->crtc_offset,
@@ -172,6 +184,7 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
uint32_t dac2_cntl;
@@ -183,11 +196,14 @@ static void legacy_crtc_load_lut(struct drm_crtc *crtc)
WREG32(RADEON_DAC_CNTL2, dac2_cntl);
WREG8(RADEON_PALETTE_INDEX, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(RADEON_PALETTE_30_DATA,
- (radeon_crtc->lut_r[i] << 20) |
- (radeon_crtc->lut_g[i] << 10) |
- (radeon_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
}
@@ -209,41 +225,10 @@ void radeon_crtc_load_lut(struct drm_crtc *crtc)
legacy_crtc_load_lut(crtc);
}
-/** Sets the color ramps on behalf of fbcon */
-void radeon_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-
- radeon_crtc->lut_r[regno] = red >> 6;
- radeon_crtc->lut_g[regno] = green >> 6;
- radeon_crtc->lut_b[regno] = blue >> 6;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno)
-{
- struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-
- *red = radeon_crtc->lut_r[regno] << 6;
- *green = radeon_crtc->lut_g[regno] << 6;
- *blue = radeon_crtc->lut_b[regno] << 6;
-}
-
static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
- int i;
-
- /* userspace palettes are always correct as is */
- for (i = 0; i < size; i++) {
- radeon_crtc->lut_r[i] = red[i] >> 6;
- radeon_crtc->lut_g[i] = green[i] >> 6;
- radeon_crtc->lut_b[i] = blue[i] >> 6;
- }
radeon_crtc_load_lut(crtc);
return 0;
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 356ad90..638bcb55 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -332,8 +332,6 @@ static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev *rfb
}
static const struct drm_fb_helper_funcs radeon_fb_helper_funcs = {
- .gamma_set = radeon_crtc_fb_gamma_set,
- .gamma_get = radeon_crtc_fb_gamma_get,
.fb_probe = radeonfb_create,
};
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
index ce6cb66..1f1856e 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
@@ -1116,7 +1116,6 @@ static const struct drm_crtc_helper_funcs legacy_helper_funcs = {
.mode_set_base_atomic = radeon_crtc_set_base_atomic,
.prepare = radeon_crtc_prepare,
.commit = radeon_crtc_commit,
- .load_lut = radeon_crtc_load_lut,
.disable = radeon_crtc_disable
};
--
2.1.4
^ permalink raw reply related
* [PATCH 10/11] drm: stm: remove dead code and pointless local lut storage
From: Peter Rosin @ 2017-06-20 19:25 UTC (permalink / raw)
To: linux-kernel
Cc: Patrik Jakobsson, Boris Brezillon, amd-gfx, intel-gfx,
David Airlie, nouveau, dri-devel, Philippe Cornu, Jani Nikula,
Christian König, Yannick Fertre, Sean Paul,
Benjamin Gaignard, Daniel Vetter, Alex Deucher, Dave Airlie,
virtualization, Vincent Abriou, Peter Rosin, Ben Skeggs
In-Reply-To: <1497986735-14418-1-git-send-email-peda@axentia.se>
The redundant fb helper .load_lut is no longer used, and can not
work right without also providing the fb helpers .gamma_set and
.gamma_get thus rendering the code in this driver suspect.
Just remove the dead code.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/stm/ltdc.c | 12 ------------
drivers/gpu/drm/stm/ltdc.h | 1 -
2 files changed, 13 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 1b9483d..87829b9 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
* DRM_CRTC
*/
-static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
-{
- struct ltdc_device *ldev = crtc_to_ltdc(crtc);
- unsigned int i, lay;
-
- for (lay = 0; lay < ldev->caps.nb_layers; lay++)
- for (i = 0; i < 256; i++)
- reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
- ldev->clut[i]);
-}
-
static void ltdc_crtc_enable(struct drm_crtc *crtc)
{
struct ltdc_device *ldev = crtc_to_ltdc(crtc);
@@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
}
static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
- .load_lut = ltdc_crtc_load_lut,
.enable = ltdc_crtc_enable,
.disable = ltdc_crtc_disable,
.mode_set_nofb = ltdc_crtc_mode_set_nofb,
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
index d7a9c73..620ca55 100644
--- a/drivers/gpu/drm/stm/ltdc.h
+++ b/drivers/gpu/drm/stm/ltdc.h
@@ -27,7 +27,6 @@ struct ltdc_device {
struct drm_panel *panel;
struct mutex err_lock; /* protecting error_status */
struct ltdc_caps caps;
- u32 clut[256]; /* color look up table */
u32 error_status;
u32 irq_status;
};
--
2.1.4
^ permalink raw reply related
* [PATCH 11/11] drm: remove unused and redundant callbacks
From: Peter Rosin @ 2017-06-20 19:25 UTC (permalink / raw)
To: linux-kernel
Cc: Patrik Jakobsson, Boris Brezillon, amd-gfx, intel-gfx,
David Airlie, nouveau, dri-devel, Philippe Cornu, Jani Nikula,
Christian König, Yannick Fertre, Sean Paul,
Benjamin Gaignard, Daniel Vetter, Alex Deucher, Dave Airlie,
virtualization, Vincent Abriou, Peter Rosin, Ben Skeggs
In-Reply-To: <1497986735-14418-1-git-send-email-peda@axentia.se>
Drivers no longer have any need for these callbacks, and there are no
users. Zap. Zap-zap-zzzap-p-pp-p.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
include/drm/drm_fb_helper.h | 32 --------------------------------
include/drm/drm_modeset_helper_vtables.h | 16 ----------------
2 files changed, 48 deletions(-)
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 119e5e4..80d9853 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -85,38 +85,6 @@ struct drm_fb_helper_surface_size {
*/
struct drm_fb_helper_funcs {
/**
- * @gamma_set:
- *
- * Set the given gamma LUT register on the given CRTC.
- *
- * This callback is optional.
- *
- * FIXME:
- *
- * This callback is functionally redundant with the core gamma table
- * support and simply exists because the fbdev hasn't yet been
- * refactored to use the core gamma table interfaces.
- */
- void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno);
- /**
- * @gamma_get:
- *
- * Read the given gamma LUT register on the given CRTC, used to save the
- * current LUT when force-restoring the fbdev for e.g. kdbg.
- *
- * This callback is optional.
- *
- * FIXME:
- *
- * This callback is functionally redundant with the core gamma table
- * support and simply exists because the fbdev hasn't yet been
- * refactored to use the core gamma table interfaces.
- */
- void (*gamma_get)(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno);
-
- /**
* @fb_probe:
*
* Driver callback to allocate and initialize the fbdev info structure.
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 85984b2..0773db9 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -267,22 +267,6 @@ struct drm_crtc_helper_funcs {
enum mode_set_atomic);
/**
- * @load_lut:
- *
- * Load a LUT prepared with the &drm_fb_helper_funcs.gamma_set vfunc.
- *
- * This callback is optional and is only used by the fbdev emulation
- * helpers.
- *
- * FIXME:
- *
- * This callback is functionally redundant with the core gamma table
- * support and simply exists because the fbdev hasn't yet been
- * refactored to use the core gamma table interfaces.
- */
- void (*load_lut)(struct drm_crtc *crtc);
-
- /**
* @disable:
*
* This callback should be used to disable the CRTC. With the atomic
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v11 4/6] mm: function to offer a page block on the free list
From: Rik van Riel @ 2017-06-20 19:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange, amit.shah, kvm, linux-kernel, liliang.opensource,
qemu-devel, virtualization, linux-mm, Dave Hansen, cornelia.huck,
pbonzini, akpm, Nitesh Narayan Lal, mgorman
In-Reply-To: <20170620212107-mutt-send-email-mst@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 1713 bytes --]
On Tue, 2017-06-20 at 21:26 +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 20, 2017 at 01:29:00PM -0400, Rik van Riel wrote:
> > I agree with that. Let me go into some more detail of
> > what Nitesh is implementing:
> >
> > 1) In arch_free_page, the being-freed page is added
> > to a per-cpu set of freed pages.
> > 2) Once that set is full, arch_free_pages goes into a
> > slow path, which:
> > 2a) Iterates over the set of freed pages, and
> > 2b) Checks whether they are still free, and
> > 2c) Adds the still free pages to a list that is
> > to be passed to the hypervisor, to be MADV_FREEd.
> > 2d) Makes that hypercall.
> >
> > Meanwhile all arch_alloc_pages has to do is make sure it
> > does not allocate a page while it is currently being
> > MADV_FREEd on the hypervisor side.
> >
> > The code Wei is working on looks like it could be
> > suitable for steps (2c) and (2d) above. Nitesh already
> > has code for steps 1 through 2b.
>
> So my question is this: Wei posted these numbers for balloon
> inflation times:
> inflating 7GB of an 8GB idle guest:
>
> 1) allocating pages (6.5%)
> 2) sending PFNs to host (68.3%)
> 3) address translation (6.1%)
> 4) madvise (19%)
>
> It takes about 4126ms for the inflating process to complete.
>
> It seems that this is an excessive amount of time to stay
> under a lock. What are your estimates for Nitesh's work?
That depends on the batch size used for step
(2c), and is something that we should be able
to tune for decent performance.
What seems to matter is that things are batched.
There are many ways to achieve that.
--
All rights reversed
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [virtio-dev] Re: [PATCH v11 6/6] virtio-balloon: VIRTIO_BALLOON_F_CMD_VQ
From: Wei Wang @ 2017-06-21 3:28 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange, virtio-dev, kvm, qemu-devel, amit.shah,
liliang.opensource, dave.hansen, linux-kernel, virtualization,
linux-mm, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <20170620190343-mutt-send-email-mst@kernel.org>
On 06/21/2017 12:18 AM, Michael S. Tsirkin wrote:
> On Fri, Jun 09, 2017 at 06:41:41PM +0800, Wei Wang wrote:
>> - if (!virtqueue_indirect_desc_table_add(vq, desc, num)) {
>> + if (!virtqueue_indirect_desc_table_add(vq, desc, *num)) {
>> virtqueue_kick(vq);
>> - wait_event(vb->acked, virtqueue_get_buf(vq, &len));
>> - vb->balloon_page_chunk.chunk_num = 0;
>> + if (busy_wait)
>> + while (!virtqueue_get_buf(vq, &len) &&
>> + !virtqueue_is_broken(vq))
>> + cpu_relax();
>> + else
>> + wait_event(vb->acked, virtqueue_get_buf(vq, &len));
>
> This is something I didn't previously notice.
> As you always keep a single buffer in flight, you do not
> really need indirect at all. Just add all descriptors
> in the ring directly, then kick.
>
> E.g.
> virtqueue_add_first
> virtqueue_add_next
> virtqueue_add_last
>
> ?
>
> You also want a flag to avoid allocations but there's no need to do it
> per descriptor, set it on vq.
>
Without using the indirect table, I'm thinking about changing to use
the standard sg (i.e. struct scatterlist), instead of vring_desc, so that
we don't need to modify or add any new functions of virtqueue_add().
In this case, we will kmalloc an array of sgs in probe(), and we can add
the sgs one by one to the vq, which won't trigger the allocation of an
indirect table inside virtqueue_add(), and then kick when all are added.
Best,
Wei
^ permalink raw reply
* Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
From: Daniel Vetter @ 2017-06-21 7:38 UTC (permalink / raw)
To: Peter Rosin
Cc: nouveau, intel-gfx, linux-kernel, dri-devel, Philippe Cornu,
Yannick Fertre, Ben Skeggs, amd-gfx, Dave Airlie, Alex Deucher,
Daniel Vetter, virtualization, Vincent Abriou,
Christian König
In-Reply-To: <1497986735-14418-2-git-send-email-peda@axentia.se>
On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> totally obsolete.
>
> I think the gamma_store can end up invalid on error. But the way I read
> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> this pesky legacy fbdev stuff be any better?
>
> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
> it saves it to the gamma_store which should already be up to date with what
> .gamma_get would return and is thus a nop. So, zap it.
Removing drm_fb_helper_save_lut_atomic should be a separate patch I
think.
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 131 ++++++++++++----------------------------
> 1 file changed, 40 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 574af01..cc2d55d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> }
> EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
>
> -static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct drm_fb_helper *helper)
> -{
> - uint16_t *r_base, *g_base, *b_base;
> - int i;
> -
> - if (helper->funcs->gamma_get == NULL)
> - return;
> -
> - r_base = crtc->gamma_store;
> - g_base = r_base + crtc->gamma_size;
> - b_base = g_base + crtc->gamma_size;
> -
> - for (i = 0; i < crtc->gamma_size; i++)
> - helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i], &b_base[i], i);
> -}
> -
> static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
> {
> uint16_t *r_base, *g_base, *b_base;
> @@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
> if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
> continue;
>
> - drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
> funcs->mode_set_base_atomic(mode_set->crtc,
> mode_set->fb,
> mode_set->x,
> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
> }
> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>
> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> - u16 blue, u16 regno, struct fb_info *info)
> -{
> - struct drm_fb_helper *fb_helper = info->par;
> - struct drm_framebuffer *fb = fb_helper->fb;
> -
> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
This case here seems gone, and it was also the pièce de résistance when I
tried tackling fbdev lut support. As far as I understand this stuff we do
not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
pointless. But I'm honestly not really clear.
I think removing this case should also be a separate patch, and I'd very
much prefer that someone with some fbdev-clue would ack it.
> - u32 *palette;
> - u32 value;
> - /* place color in psuedopalette */
> - if (regno > 16)
> - return -EINVAL;
> - palette = (u32 *)info->pseudo_palette;
> - red >>= (16 - info->var.red.length);
> - green >>= (16 - info->var.green.length);
> - blue >>= (16 - info->var.blue.length);
> - value = (red << info->var.red.offset) |
> - (green << info->var.green.offset) |
> - (blue << info->var.blue.offset);
> - if (info->var.transp.length > 0) {
> - u32 mask = (1 << info->var.transp.length) - 1;
> -
> - mask <<= info->var.transp.offset;
> - value |= mask;
> - }
> - palette[regno] = value;
> - return 0;
> - }
> -
> - /*
> - * The driver really shouldn't advertise pseudo/directcolor
> - * visuals if it can't deal with the palette.
> - */
> - if (WARN_ON(!fb_helper->funcs->gamma_set ||
> - !fb_helper->funcs->gamma_get))
> - return -EINVAL;
> -
> - WARN_ON(fb->format->cpp[0] != 1);
> -
> - fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> -
> - return 0;
> -}
> -
> /**
> * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
> * @cmap: cmap to set
> @@ -1220,51 +1159,61 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
> {
> struct drm_fb_helper *fb_helper = info->par;
> struct drm_device *dev = fb_helper->dev;
> - const struct drm_crtc_helper_funcs *crtc_funcs;
> - u16 *red, *green, *blue, *transp;
> + struct drm_modeset_acquire_ctx ctx;
> struct drm_crtc *crtc;
> - int i, j, rc = 0;
> - int start;
> + u16 *r, *g, *b;
> + int i, ret;
>
> if (oops_in_progress)
> return -EBUSY;
>
> - drm_modeset_lock_all(dev);
> + if (cmap->start + cmap->len < cmap->start)
> + return -EINVAL;
> +
> + drm_modeset_acquire_init(&ctx, 0);
> +retry:
> + ret = drm_modeset_lock_all_ctx(dev, &ctx);
> + if (ret)
> + goto out;
> if (!drm_fb_helper_is_bound(fb_helper)) {
> - drm_modeset_unlock_all(dev);
> - return -EBUSY;
> + ret = -EBUSY;
> + goto out;
> }
>
> for (i = 0; i < fb_helper->crtc_count; i++) {
> crtc = fb_helper->crtc_info[i].mode_set.crtc;
> - crtc_funcs = crtc->helper_private;
> -
> - red = cmap->red;
> - green = cmap->green;
> - blue = cmap->blue;
> - transp = cmap->transp;
> - start = cmap->start;
> + if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
> + ret = -EINVAL;
> + goto out;
> + }
>
> - for (j = 0; j < cmap->len; j++) {
> - u16 hred, hgreen, hblue, htransp = 0xffff;
> + if (cmap->start + cmap->len > crtc->gamma_size) {
> + ret = -EINVAL;
> + goto out;
> + }
>
> - hred = *red++;
> - hgreen = *green++;
> - hblue = *blue++;
> + r = crtc->gamma_store;
> + g = r + crtc->gamma_size;
> + b = g + crtc->gamma_size;
>
> - if (transp)
> - htransp = *transp++;
> + memcpy(r + cmap->start, cmap->red, cmap->len * sizeof(u16));
> + memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(u16));
> + memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(u16));
>
> - rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
> - if (rc)
> - goto out;
> - }
> - if (crtc_funcs->load_lut)
> - crtc_funcs->load_lut(crtc);
> + ret = crtc->funcs->gamma_set(crtc, r, g, b,
> + crtc->gamma_size, &ctx);
> + if (ret)
> + break;
> }
> - out:
> - drm_modeset_unlock_all(dev);
> - return rc;
> +out:
> + if (ret == -EDEADLK) {
> + drm_modeset_backoff(&ctx);
> + goto retry;
> + }
> + drm_modeset_drop_locks(&ctx);
> + drm_modeset_acquire_fini(&ctx);
> +
> + return ret;
It's a pre-existing bug, but should we also try to restore the fbdev lut
in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug,
but might be relevant for your use-case. Just try to run both an fbdev
application and some kms-native thing, and then SIGKILL the native kms
app.
But since pre-existing not really required, and probably too much effort.
Thanks, Daniel
> }
> EXPORT_SYMBOL(drm_fb_helper_setcmap);
>
> --
> 2.1.4
>
> _______________________________________________
> 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
^ permalink raw reply
* Re: [PATCH 00/11] improve the fb_setcmap helper
From: Daniel Vetter @ 2017-06-21 7:40 UTC (permalink / raw)
To: Peter Rosin
Cc: nouveau, intel-gfx, linux-kernel, dri-devel, Philippe Cornu,
Yannick Fertre, Ben Skeggs, amd-gfx, Dave Airlie, Alex Deucher,
Daniel Vetter, virtualization, Vincent Abriou,
Christian König
In-Reply-To: <1497986735-14418-1-git-send-email-peda@axentia.se>
On Tue, Jun 20, 2017 at 09:25:24PM +0200, Peter Rosin wrote:
> Hi!
>
> While trying to get CLUT support for the atmel_hlcdc driver, and
> specifically for the emulated fbdev interface, I received some
> push-back that my feeble in-driver attempts should be solved
> by the core. This is my attempt to do it right.
>
> Boris and Daniel, was this approximately what you had in mind?
Yeah, this is awesome. I tried to do it a few times myself, but always
failed (also due to lack of real use-case on my side).
> I have obviously not tested all of this with more than a compile,
> but the first patch is enough to make the atmel-hlcdc driver
> do what I need. The rest is just lots of removals and cleanup
> made possible by the improved core.
If it works for you it's imo good enough. Not sure anyone else really
cares about fbdev lut support at all. I have a few comments on the first
patch, but once that's sorted, and once we have given driver maintainers
enough time to ack I think I'll merge the entire pile into drm-misc.
Nice work, thanks for doing it.
Cheers, Daniel
> Please test, I would not be surprised if I have fouled up some
> bit-manipulation somewhere in this mostly mechanical change...
>
> Cheers,
> peda
>
> Peter Rosin (11):
> drm/fb-helper: do a generic fb_setcmap helper in terms of crtc
> .gamma_set
> drm: amd: remove dead code and pointless local lut storage
> drm: ast: remove dead code and pointless local lut storage
> drm: cirrus: remove dead code and pointless local lut storage
> dmr: gma500: remove dead code and pointless local lut storage
> drm: i915: remove dead code and pointless local lut storage
> drm: mgag200: remove dead code and pointless local lut storage
> drm: nouveau: remove dead code and pointless local lut storage
> drm: radeon: remove dead code and pointless local lut storage
> drm: stm: remove dead code and pointless local lut storage
> drm: remove unused and redundant callbacks
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 24 -----
> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 -
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 27 ++----
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 27 ++----
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 27 ++----
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 27 ++----
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 23 -----
> drivers/gpu/drm/ast/ast_drv.h | 1 -
> drivers/gpu/drm/ast/ast_fb.c | 20 -----
> drivers/gpu/drm/ast/ast_mode.c | 26 ++----
> drivers/gpu/drm/cirrus/cirrus_drv.h | 8 --
> drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 -
> drivers/gpu/drm/cirrus/cirrus_mode.c | 71 ++++-----------
> drivers/gpu/drm/drm_fb_helper.c | 131 +++++++++-------------------
> drivers/gpu/drm/gma500/framebuffer.c | 22 -----
> drivers/gpu/drm/gma500/gma_display.c | 32 +++----
> drivers/gpu/drm/gma500/psb_intel_display.c | 7 +-
> drivers/gpu/drm/gma500/psb_intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> drivers/gpu/drm/i915/intel_fbdev.c | 31 -------
> drivers/gpu/drm/mgag200/mgag200_drv.h | 5 --
> drivers/gpu/drm/mgag200/mgag200_fb.c | 2 -
> drivers/gpu/drm/mgag200/mgag200_mode.c | 62 ++++---------
> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 26 ++----
> drivers/gpu/drm/nouveau/nouveau_crtc.h | 3 -
> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 22 -----
> drivers/gpu/drm/nouveau/nv50_display.c | 39 +++------
> drivers/gpu/drm/radeon/atombios_crtc.c | 1 -
> drivers/gpu/drm/radeon/radeon_connectors.c | 7 +-
> drivers/gpu/drm/radeon/radeon_display.c | 71 ++++++---------
> drivers/gpu/drm/radeon/radeon_fb.c | 2 -
> drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 1 -
> drivers/gpu/drm/stm/ltdc.c | 12 ---
> drivers/gpu/drm/stm/ltdc.h | 1 -
> include/drm/drm_fb_helper.h | 32 -------
> include/drm/drm_modeset_helper_vtables.h | 16 ----
> 36 files changed, 171 insertions(+), 640 deletions(-)
>
> --
> 2.1.4
>
> _______________________________________________
> 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
^ permalink raw reply
* Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
From: Michel Dänzer @ 2017-06-21 7:59 UTC (permalink / raw)
To: Peter Rosin, Philippe Cornu, Christian König, Yannick Fertre,
Gerd Hoffmann, Daniel Vetter, Alex Deucher, Dave Airlie,
Vincent Abriou, Ben Skeggs
Cc: nouveau, intel-gfx, linux-kernel, dri-devel, virtualization,
amd-gfx
In-Reply-To: <20170621073804.akyg4rxvoavjjt2v@phenom.ffwll.local>
On 21/06/17 04:38 PM, Daniel Vetter wrote:
> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
>> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>> totally obsolete.
>>
>> I think the gamma_store can end up invalid on error. But the way I read
>> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
>> this pesky legacy fbdev stuff be any better?
>>
>> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
>> it saves it to the gamma_store which should already be up to date with what
>> .gamma_get would return and is thus a nop. So, zap it.
>
> Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> think.
>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
[...]
>> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
>> }
>> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>>
>> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>> - u16 blue, u16 regno, struct fb_info *info)
>> -{
>> - struct drm_fb_helper *fb_helper = info->par;
>> - struct drm_framebuffer *fb = fb_helper->fb;
>> -
>> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
>
> This case here seems gone, and it was also the pièce de résistance when I
> tried tackling fbdev lut support. As far as I understand this stuff we do
> not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
> pointless. But I'm honestly not really clear.
>
> I think removing this case should also be a separate patch, and I'd very
> much prefer that someone with some fbdev-clue would ack it.
No need for anyone with fbdev-clue, just run fbset -i. :) fbcon uses a
TRUECOLOR visual at depths > 8.
> It's a pre-existing bug, but should we also try to restore the fbdev lut
> in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug,
> but might be relevant for your use-case. Just try to run both an fbdev
> application and some kms-native thing, and then SIGKILL the native kms
> app.
>
> But since pre-existing not really required, and probably too much effort.
I suspect something like that indeed needs to be done though. E.g.
killing Xorg results in fbcon continuing to use the LUT set by Xorg.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
From: Daniel Vetter @ 2017-06-21 8:14 UTC (permalink / raw)
To: Michel Dänzer
Cc: amd-gfx, nouveau, intel-gfx, Philippe Cornu, dri-devel,
Peter Rosin, Yannick Fertre, Dave Airlie, Alex Deucher,
Daniel Vetter, virtualization, Vincent Abriou,
Christian König, linux-kernel, Ben Skeggs
In-Reply-To: <dfd961c1-2ca9-d85e-fc28-d873666b9bf3@daenzer.net>
On Wed, Jun 21, 2017 at 04:59:31PM +0900, Michel Dänzer wrote:
> On 21/06/17 04:38 PM, Daniel Vetter wrote:
> > On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
> >> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> >> totally obsolete.
> >>
> >> I think the gamma_store can end up invalid on error. But the way I read
> >> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> >> this pesky legacy fbdev stuff be any better?
> >>
> >> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
> >> it saves it to the gamma_store which should already be up to date with what
> >> .gamma_get would return and is thus a nop. So, zap it.
> >
> > Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> > think.
> >
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
>
> [...]
>
> >> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
> >> }
> >> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
> >>
> >> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> >> - u16 blue, u16 regno, struct fb_info *info)
> >> -{
> >> - struct drm_fb_helper *fb_helper = info->par;
> >> - struct drm_framebuffer *fb = fb_helper->fb;
> >> -
> >> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> >
> > This case here seems gone, and it was also the pièce de résistance when I
> > tried tackling fbdev lut support. As far as I understand this stuff we do
> > not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
> > pointless. But I'm honestly not really clear.
> >
> > I think removing this case should also be a separate patch, and I'd very
> > much prefer that someone with some fbdev-clue would ack it.
>
> No need for anyone with fbdev-clue, just run fbset -i. :) fbcon uses a
> TRUECOLOR visual at depths > 8.
Then I understand even less what exactly this code does ... Should we keep
it? Is it just pure cargo-cult? Only needed when we'd change the color
depth of the fbdev buffer, which we never do at runtime?
> > It's a pre-existing bug, but should we also try to restore the fbdev lut
> > in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug,
> > but might be relevant for your use-case. Just try to run both an fbdev
> > application and some kms-native thing, and then SIGKILL the native kms
> > app.
> >
> > But since pre-existing not really required, and probably too much effort.
>
> I suspect something like that indeed needs to be done though. E.g.
> killing Xorg results in fbcon continuing to use the LUT set by Xorg.
Ok, the only trouble with that is atomic kms locking, which requires that
we can only do 1 commit per lock-holding time, and also
drm_modeset_lock_all is an evil hack and needs to be phased out. Two
solutions:
- We just update the lut after we've dropped the locks again in
restore_fbdev_mode.
- We need both a legacy path, in restore_fbdev_mode_legacy, and an atomic
path in restore_fbdev_mode_atomic. The latter cannot use the gamme_set
callback, since that would call drm_atomic_helper_legacy_gamma_set for
atomic drivers, which would result in two calls to drm_atomic_commit in
one critical sections. Instead that needs to open-code the logic in
drm_atomic_helper_legacy_gamma_set and integrate it into the overall
fbdev restore commit.
The 2nd option is a bit more typing, but much cleaner. It also fits better
into some of the refactoring plans I have (I want to get rid of
drm_modeset_lock_all in the fbdev emulation). And has the benefit of
giving drivers a bit more complex fbdev emulation atomic commits, which
would be good for testing I think.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
From: Michel Dänzer @ 2017-06-21 8:36 UTC (permalink / raw)
To: Peter Rosin, Philippe Cornu, Christian König, Yannick Fertre,
Gerd Hoffmann, Daniel Vetter, Alex Deucher, Dave Airlie,
Vincent Abriou, Ben Skeggs, Bartlomiej Zolnierkiewicz
Cc: linux-fbdev, nouveau, intel-gfx, linux-kernel, amd-gfx,
virtualization, dri-devel
In-Reply-To: <20170621081454.ymykfka5peelkqxp@phenom.ffwll.local>
On 21/06/17 05:14 PM, Daniel Vetter wrote:
> On Wed, Jun 21, 2017 at 04:59:31PM +0900, Michel Dänzer wrote:
>> On 21/06/17 04:38 PM, Daniel Vetter wrote:
>>> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
>>>> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>>>> totally obsolete.
>>>>
>>>> I think the gamma_store can end up invalid on error. But the way I read
>>>> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
>>>> this pesky legacy fbdev stuff be any better?
>>>>
>>>> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
>>>> it saves it to the gamma_store which should already be up to date with what
>>>> .gamma_get would return and is thus a nop. So, zap it.
>>>
>>> Removing drm_fb_helper_save_lut_atomic should be a separate patch I
>>> think.
>>>
>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>
>> [...]
>>
>>>> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
>>>> }
>>>> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>>>>
>>>> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>>>> - u16 blue, u16 regno, struct fb_info *info)
>>>> -{
>>>> - struct drm_fb_helper *fb_helper = info->par;
>>>> - struct drm_framebuffer *fb = fb_helper->fb;
>>>> -
>>>> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
>>>
>>> This case here seems gone, and it was also the pièce de résistance when I
>>> tried tackling fbdev lut support. As far as I understand this stuff we do
>>> not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
>>> pointless. But I'm honestly not really clear.
>>>
>>> I think removing this case should also be a separate patch, and I'd very
>>> much prefer that someone with some fbdev-clue would ack it.
>>
>> No need for anyone with fbdev-clue, just run fbset -i. :)
Adding Bartlomiej and the linux-fbdev list, just in case I'm wrong below.
>> fbcon uses a TRUECOLOR visual at depths > 8.
>
> Then I understand even less what exactly this code does ... Should we keep
> it?
I think we need it. It updates the entries of info->pseudo_palette,
which is kind of a pseudocolor palette mapping 16 indices to pixel
values to write to the framebuffer.
If the setcolreg hook is removed, the setcmap hook needs to do this instead.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v11 4/6] mm: function to offer a page block on the free list
From: Wei Wang @ 2017-06-21 8:38 UTC (permalink / raw)
To: Rik van Riel, David Hildenbrand, Dave Hansen, linux-kernel,
qemu-devel, virtualization, kvm, linux-mm, mst, cornelia.huck,
akpm, mgorman, aarcange, amit.shah, pbonzini, liliang.opensource
Cc: Nitesh Narayan Lal
In-Reply-To: <1497979740.20270.102.camel@redhat.com>
On 06/21/2017 01:29 AM, Rik van Riel wrote:
> On Tue, 2017-06-20 at 18:49 +0200, David Hildenbrand wrote:
>> On 20.06.2017 18:44, Rik van Riel wrote:
>>> Nitesh Lal (on the CC list) is working on a way
>>> to efficiently batch recently freed pages for
>>> free page hinting to the hypervisor.
>>>
>>> If that is done efficiently enough (eg. with
>>> MADV_FREE on the hypervisor side for lazy freeing,
>>> and lazy later re-use of the pages), do we still
>>> need the harder to use batch interface from this
>>> patch?
>>>
>> David's opinion incoming:
>>
>> No, I think proper free page hinting would be the optimum solution,
>> if
>> done right. This would avoid the batch interface and even turn
>> virtio-balloon in some sense useless.
> I agree with that. Let me go into some more detail of
> what Nitesh is implementing:
>
> 1) In arch_free_page, the being-freed page is added
> to a per-cpu set of freed pages.
I got some questions here:
1. Are the pages managed one by one on the per-CPU set?
For example, when there are 2 adjacent pages, are they still
put as two nodes on the per-CPU list? or the buddy algorithm
will be re-implemented on the per-CPU list as well?
2. Looks like this will be added to the common free function.
Normally, people may not need the free page hint, do they
need to carry the added burden?
> 2) Once that set is full, arch_free_pages goes into a
> slow path, which:
> 2a) Iterates over the set of freed pages, and
> 2b) Checks whether they are still free, and
The pages that have been double checked as "free"
pages here and added to the list for the hypervisor can
also be immediately used.
> 2c) Adds the still free pages to a list that is
> to be passed to the hypervisor, to be MADV_FREEd.
> 2d) Makes that hypercall.
>
> Meanwhile all arch_alloc_pages has to do is make sure it
> does not allocate a page while it is currently being
> MADV_FREEd on the hypervisor side.
Is this proposed to replace the balloon driver?
>
> The code Wei is working on looks like it could be
> suitable for steps (2c) and (2d) above. Nitesh already
> has code for steps 1 through 2b.
>
May I know the advantages of the added steps? Thanks.
Best,
Wei
^ permalink raw reply
* Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
From: Peter Rosin @ 2017-06-21 9:40 UTC (permalink / raw)
To: linux-kernel, amd-gfx, intel-gfx, nouveau, dri-devel,
Philippe Cornu, Christian König, Yannick Fertre,
Gerd Hoffmann, Daniel Vetter, Alex Deucher, Dave Airlie,
virtualization, Vincent Abriou, Ben Skeggs
In-Reply-To: <20170621073804.akyg4rxvoavjjt2v@phenom.ffwll.local>
On 2017-06-21 09:38, Daniel Vetter wrote:
> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
>> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
>> totally obsolete.
>>
>> I think the gamma_store can end up invalid on error. But the way I read
>> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
>> this pesky legacy fbdev stuff be any better?
>>
>> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
>> it saves it to the gamma_store which should already be up to date with what
>> .gamma_get would return and is thus a nop. So, zap it.
>
> Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> think.
Then 3 patches would be needed, first some hybrid thing that does it the
old way, but also stores the lut in .gamma_store, then the split-out that
removes drm_fb_helper_save_lut_atomic, then whatever is needed to get
to the desired code. I can certainly do that, but do you want me to?
I.e., the statement that drm_fb_helper_save_lut_atomic is a nop is only
true when (part of) the other patch is also considered.
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>
>> ---
>> drivers/gpu/drm/drm_fb_helper.c | 131 ++++++++++++----------------------------
>> 1 file changed, 40 insertions(+), 91 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 574af01..cc2d55d 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
>> }
>> EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
>>
>> -static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct drm_fb_helper *helper)
>> -{
>> - uint16_t *r_base, *g_base, *b_base;
>> - int i;
>> -
>> - if (helper->funcs->gamma_get == NULL)
>> - return;
>> -
>> - r_base = crtc->gamma_store;
>> - g_base = r_base + crtc->gamma_size;
>> - b_base = g_base + crtc->gamma_size;
>> -
>> - for (i = 0; i < crtc->gamma_size; i++)
>> - helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i], &b_base[i], i);
>> -}
>> -
>> static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
>> {
>> uint16_t *r_base, *g_base, *b_base;
>> @@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
>> if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
>> continue;
>>
>> - drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
>> funcs->mode_set_base_atomic(mode_set->crtc,
>> mode_set->fb,
>> mode_set->x,
>> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper,
>> }
>> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>>
>> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>> - u16 blue, u16 regno, struct fb_info *info)
>> -{
>> - struct drm_fb_helper *fb_helper = info->par;
>> - struct drm_framebuffer *fb = fb_helper->fb;
>> -
>> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
>
> This case here seems gone, and it was also the pièce de résistance when I
> tried tackling fbdev lut support. As far as I understand this stuff we do
> not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
> pointless. But I'm honestly not really clear.
Oops, sorry, I simply missed that, I'll have a closer look...
> I think removing this case should also be a separate patch, and I'd very
> much prefer that someone with some fbdev-clue would ack it.
>
>> - u32 *palette;
>> - u32 value;
>> - /* place color in psuedopalette */
>> - if (regno > 16)
>> - return -EINVAL;
>> - palette = (u32 *)info->pseudo_palette;
>> - red >>= (16 - info->var.red.length);
>> - green >>= (16 - info->var.green.length);
>> - blue >>= (16 - info->var.blue.length);
>> - value = (red << info->var.red.offset) |
>> - (green << info->var.green.offset) |
>> - (blue << info->var.blue.offset);
>> - if (info->var.transp.length > 0) {
>> - u32 mask = (1 << info->var.transp.length) - 1;
>> -
>> - mask <<= info->var.transp.offset;
>> - value |= mask;
>> - }
>> - palette[regno] = value;
>> - return 0;
>> - }
>> -
>> - /*
>> - * The driver really shouldn't advertise pseudo/directcolor
>> - * visuals if it can't deal with the palette.
>> - */
>> - if (WARN_ON(!fb_helper->funcs->gamma_set ||
>> - !fb_helper->funcs->gamma_get))
>> - return -EINVAL;
>> -
>> - WARN_ON(fb->format->cpp[0] != 1);
>> -
>> - fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
>> -
>> - return 0;
>> -}
>> -
>> /**
>> * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
>> * @cmap: cmap to set
>> @@ -1220,51 +1159,61 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>> {
>> struct drm_fb_helper *fb_helper = info->par;
>> struct drm_device *dev = fb_helper->dev;
>> - const struct drm_crtc_helper_funcs *crtc_funcs;
>> - u16 *red, *green, *blue, *transp;
>> + struct drm_modeset_acquire_ctx ctx;
>> struct drm_crtc *crtc;
>> - int i, j, rc = 0;
>> - int start;
>> + u16 *r, *g, *b;
>> + int i, ret;
>>
>> if (oops_in_progress)
>> return -EBUSY;
>>
>> - drm_modeset_lock_all(dev);
>> + if (cmap->start + cmap->len < cmap->start)
>> + return -EINVAL;
>> +
>> + drm_modeset_acquire_init(&ctx, 0);
>> +retry:
>> + ret = drm_modeset_lock_all_ctx(dev, &ctx);
>> + if (ret)
>> + goto out;
>> if (!drm_fb_helper_is_bound(fb_helper)) {
>> - drm_modeset_unlock_all(dev);
>> - return -EBUSY;
>> + ret = -EBUSY;
>> + goto out;
>> }
>>
>> for (i = 0; i < fb_helper->crtc_count; i++) {
>> crtc = fb_helper->crtc_info[i].mode_set.crtc;
>> - crtc_funcs = crtc->helper_private;
>> -
>> - red = cmap->red;
>> - green = cmap->green;
>> - blue = cmap->blue;
>> - transp = cmap->transp;
>> - start = cmap->start;
>> + if (!crtc->funcs->gamma_set || !crtc->gamma_size) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>>
>> - for (j = 0; j < cmap->len; j++) {
>> - u16 hred, hgreen, hblue, htransp = 0xffff;
>> + if (cmap->start + cmap->len > crtc->gamma_size) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>>
>> - hred = *red++;
>> - hgreen = *green++;
>> - hblue = *blue++;
>> + r = crtc->gamma_store;
>> + g = r + crtc->gamma_size;
>> + b = g + crtc->gamma_size;
>>
>> - if (transp)
>> - htransp = *transp++;
>> + memcpy(r + cmap->start, cmap->red, cmap->len * sizeof(u16));
>> + memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(u16));
>> + memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(u16));
>>
>> - rc = setcolreg(crtc, hred, hgreen, hblue, start++, info);
>> - if (rc)
>> - goto out;
>> - }
>> - if (crtc_funcs->load_lut)
>> - crtc_funcs->load_lut(crtc);
>> + ret = crtc->funcs->gamma_set(crtc, r, g, b,
>> + crtc->gamma_size, &ctx);
>> + if (ret)
>> + break;
>> }
>> - out:
>> - drm_modeset_unlock_all(dev);
>> - return rc;
>> +out:
>> + if (ret == -EDEADLK) {
>> + drm_modeset_backoff(&ctx);
>> + goto retry;
>> + }
>> + drm_modeset_drop_locks(&ctx);
>> + drm_modeset_acquire_fini(&ctx);
>> +
>> + return ret;
>
> It's a pre-existing bug, but should we also try to restore the fbdev lut
> in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug,
> but might be relevant for your use-case. Just try to run both an fbdev
> application and some kms-native thing, and then SIGKILL the native kms
> app.
>
> But since pre-existing not really required, and probably too much effort.
Good thing too, because I don't really know my way around this code...
Cheers,
peda
> Thanks, Daniel
>
>> }
>> EXPORT_SYMBOL(drm_fb_helper_setcmap);
>>
>> --
>> 2.1.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 08/11] drm: nouveau: remove dead code and pointless local lut storage
From: Peter Rosin @ 2017-06-21 10:35 UTC (permalink / raw)
To: linux-kernel
Cc: Boris Brezillon, amd-gfx, intel-gfx, David Airlie, nouveau,
dri-devel, Philippe Cornu, Jani Nikula, Patrik Jakobsson,
Yannick Fertre, Sean Paul, Benjamin Gaignard, Dave Airlie,
Alex Deucher, Daniel Vetter, virtualization, Vincent Abriou,
Christian König, Ben Skeggs
In-Reply-To: <1497986735-14418-9-git-send-email-peda@axentia.se>
On 2017-06-20 21:25, Peter Rosin wrote:
> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
> no longer used. Remove the dead code and hook up the crtc .gamma_set
> to use the crtc gamma_store directly instead of duplicating that
> info locally.
[...]
> - for (i = 0; i < 256; i++) {
> - u16 r = nv_crtc->lut.r[i] >> 2;
> - u16 g = nv_crtc->lut.g[i] >> 2;
> - u16 b = nv_crtc->lut.b[i] >> 2;
> + r = crtc->gamma_store;
> + g = r + crtc->gamma_size;
> + b = g + crtc->gamma_size;
>
> + for (i = 0; i < 256; i++) {
> if (disp->disp->oclass < GF110_DISP) {
> - writew(r + 0x0000, lut + (i * 0x08) + 0);
> - writew(g + 0x0000, lut + (i * 0x08) + 2);
> - writew(b + 0x0000, lut + (i * 0x08) + 4);
> + writew((*r++ >> 2) + 0x0000, lut + (i * 0x08) + 0);
> + writew((*g++ >> 2) + 0x0000, lut + (i * 0x08) + 2);
> + writew((*b++ >> 2) + 0x0000, lut + (i * 0x08) + 4);
> } else {
> - writew(r + 0x6000, lut + (i * 0x20) + 0);
> - writew(g + 0x6000, lut + (i * 0x20) + 2);
> - writew(b + 0x6000, lut + (i * 0x20) + 4);
> + writew((*r++ >> 2) + 0x6000, lut + (i * 0x20) + 0);
> + writew((*g++ >> 2) + 0x6000, lut + (i * 0x20) + 2);
> + writew((*b++ >> 2) + 0x6000, lut + (i * 0x20) + 4);
> }
> }
> }
I forgot to mention this, but the above is very strange for
disp->disp->oclass >= GF110_DISP because 0x6000 interferes with
the 14 bits that appear to be the lut depth in the registers.
I suspect some other bit-shift should be used for that case?
Someone should probably consult a datasheet...
Cheers,
peda
^ permalink raw reply
* Re: [RFC] virtio-mem: paravirtualized memory
From: Stefan Hajnoczi @ 2017-06-21 11:08 UTC (permalink / raw)
To: David Hildenbrand
Cc: Andrea Arcangeli, KVM, Michael S. Tsirkin, qemu-devel@nongnu.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org
In-Reply-To: <4cec825b-d92e-832e-3a76-103767032528@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 1509 bytes --]
On Mon, Jun 19, 2017 at 12:26:52PM +0200, David Hildenbrand wrote:
> On 19.06.2017 12:08, Stefan Hajnoczi wrote:
> > On Fri, Jun 16, 2017 at 04:20:02PM +0200, David Hildenbrand wrote:
> >> Important restrictions of this concept:
> >> - Guests without a virtio-mem guest driver can't see that memory.
> >> - We will always require some boot memory that cannot get unplugged.
> >> Also, virtio-mem memory (as all other hotplugged memory) cannot become
> >> DMA memory under Linux. So the boot memory also defines the amount of
> >> DMA memory.
> >
> > I didn't know that hotplug memory cannot become DMA memory.
> >
> > Ouch. Zero-copy disk I/O with O_DIRECT and network I/O with virtio-net
> > won't be possible.
> >
> > When running an application that uses O_DIRECT file I/O this probably
> > means we now have 2 copies of pages in memory: 1. in the application and
> > 2. in the kernel page cache.
> >
> > So this increases pressure on the page cache and reduces performance :(.
> >
> > Stefan
> >
>
> arch/x86/mm/init_64.c:
>
> /*
> * Memory is added always to NORMAL zone. This means you will never get
> * additional DMA/DMA32 memory.
> */
> int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> {
>
> The is for sure something to work on in the future. Until then, base
> memory of 3.X GB should be sufficient, right?
I'm not sure that helps because applications typically don't control
where their buffers are located?
Stefan
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [virtio-dev] Re: [PATCH v11 6/6] virtio-balloon: VIRTIO_BALLOON_F_CMD_VQ
From: Michael S. Tsirkin @ 2017-06-21 12:28 UTC (permalink / raw)
To: Wei Wang
Cc: aarcange, virtio-dev, riel, kvm, qemu-devel, amit.shah,
liliang.opensource, dave.hansen, linux-kernel, virtualization,
linux-mm, cornelia.huck, pbonzini, akpm, nilal, mgorman
In-Reply-To: <5949E7C0.3050106@intel.com>
On Wed, Jun 21, 2017 at 11:28:00AM +0800, Wei Wang wrote:
> On 06/21/2017 12:18 AM, Michael S. Tsirkin wrote:
> > On Fri, Jun 09, 2017 at 06:41:41PM +0800, Wei Wang wrote:
> > > - if (!virtqueue_indirect_desc_table_add(vq, desc, num)) {
> > > + if (!virtqueue_indirect_desc_table_add(vq, desc, *num)) {
> > > virtqueue_kick(vq);
> > > - wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> > > - vb->balloon_page_chunk.chunk_num = 0;
> > > + if (busy_wait)
> > > + while (!virtqueue_get_buf(vq, &len) &&
> > > + !virtqueue_is_broken(vq))
> > > + cpu_relax();
> > > + else
> > > + wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> >
> > This is something I didn't previously notice.
> > As you always keep a single buffer in flight, you do not
> > really need indirect at all. Just add all descriptors
> > in the ring directly, then kick.
> >
> > E.g.
> > virtqueue_add_first
> > virtqueue_add_next
> > virtqueue_add_last
> >
> > ?
> >
> > You also want a flag to avoid allocations but there's no need to do it
> > per descriptor, set it on vq.
> >
>
> Without using the indirect table, I'm thinking about changing to use
> the standard sg (i.e. struct scatterlist), instead of vring_desc, so that
> we don't need to modify or add any new functions of virtqueue_add().
>
> In this case, we will kmalloc an array of sgs in probe(), and we can add
> the sgs one by one to the vq, which won't trigger the allocation of an
> indirect table inside virtqueue_add(), and then kick when all are added.
>
> Best,
> Wei
And allocate headers too? This can work. API extensions aren't
necessarily a bad idea though. The API I suggest above is preferable
for the simple reason that it can work without INDIRECT flag
support in hypervisor.
I wonder which APIs would Nitesh find useful.
--
MST
^ permalink raw reply
* Re: [RFC] virtio-mem: paravirtualized memory
From: David Hildenbrand @ 2017-06-21 12:32 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Andrea Arcangeli, KVM, Michael S. Tsirkin, qemu-devel@nongnu.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org
In-Reply-To: <20170621110817.GF16183@stefanha-x1.localdomain>
On 21.06.2017 13:08, Stefan Hajnoczi wrote:
> On Mon, Jun 19, 2017 at 12:26:52PM +0200, David Hildenbrand wrote:
>> On 19.06.2017 12:08, Stefan Hajnoczi wrote:
>>> On Fri, Jun 16, 2017 at 04:20:02PM +0200, David Hildenbrand wrote:
>>>> Important restrictions of this concept:
>>>> - Guests without a virtio-mem guest driver can't see that memory.
>>>> - We will always require some boot memory that cannot get unplugged.
>>>> Also, virtio-mem memory (as all other hotplugged memory) cannot become
>>>> DMA memory under Linux. So the boot memory also defines the amount of
>>>> DMA memory.
>>>
>>> I didn't know that hotplug memory cannot become DMA memory.
>>>
>>> Ouch. Zero-copy disk I/O with O_DIRECT and network I/O with virtio-net
>>> won't be possible.
>>>
>>> When running an application that uses O_DIRECT file I/O this probably
>>> means we now have 2 copies of pages in memory: 1. in the application and
>>> 2. in the kernel page cache.
>>>
>>> So this increases pressure on the page cache and reduces performance :(.
>>>
>>> Stefan
>>>
>>
>> arch/x86/mm/init_64.c:
>>
>> /*
>> * Memory is added always to NORMAL zone. This means you will never get
>> * additional DMA/DMA32 memory.
>> */
>> int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>> {
>>
>> The is for sure something to work on in the future. Until then, base
>> memory of 3.X GB should be sufficient, right?
>
> I'm not sure that helps because applications typically don't control
> where their buffers are located?
Okay, let me try to explain what is going on here (no expert, please
someone correct me if I am wrong).
There is a difference between DMA and DMA memory in Linux. DMA memory is
simply memory with special addresses. DMA is the general technique of a
device directly copying data to ram, bypassing the CPU.
ZONE_DMA contains all* memory < 16MB
ZONE_DMA32 contains all* memory < 4G
* meaning available on boot via a820 map, not hotplugged.
So memory from these zones can be used by devices that can only deal
with 24bit/32bit addresses.
Hotplugged memory is never added to the ZONE_DMA/DMA32, but to
ZONE_NORMAL. That means, kmalloc(.., GFP_DMA will) not be able to use
hotplugged memory. Say you have 1GB of main storage and hotplug 1G (on
address 1G). This memory will not be available in the ZONE_DMA, although
below 4g.
Memory in ZONE_NORMAL is used for ordinary kmalloc(), so all these
memory can be used to do DMA, but you are not guaranteed to get 32bit
capable addresses. I pretty much assume that virtio-net can deal with
64bit addresses.
My understanding of O_DIRECT:
The user space buffers (O_DIRECT) is directly used to do DMA. This will
work just fine as long as the device can deal with 64bit addresses. I
guess this is the case for virtio-net, otherwise there would be the
exact same problem already without virtio-mem.
Summary:
virtio-mem memory can be used for DMA, it will simply not be added to
ZONE_DMA/DMA32 and therefore won't be available for kmalloc(...,
GFP_DMA). This should work just fine with O_DIRECT as before.
If necessary, we could try to add memory to the ZONE_DMA later on,
however for now I would rate this a minor problem. By simply using 3.X
GB of base memory, basically all memory that could go to ZONE_DMA/DMA32
already is in these zones without virtio-mem.
Thanks!
>
> Stefan
>
--
Thanks,
David
^ permalink raw reply
* Re: [PATCH v11 4/6] mm: function to offer a page block on the free list
From: Michael S. Tsirkin @ 2017-06-21 12:41 UTC (permalink / raw)
To: Rik van Riel
Cc: aarcange, amit.shah, kvm, linux-kernel, liliang.opensource,
qemu-devel, virtualization, linux-mm, Dave Hansen, cornelia.huck,
pbonzini, akpm, Nitesh Narayan Lal, mgorman
In-Reply-To: <1497988260.20270.109.camel@redhat.com>
On Tue, Jun 20, 2017 at 03:51:00PM -0400, Rik van Riel wrote:
> On Tue, 2017-06-20 at 21:26 +0300, Michael S. Tsirkin wrote:
> > On Tue, Jun 20, 2017 at 01:29:00PM -0400, Rik van Riel wrote:
> > > I agree with that. Let me go into some more detail of
> > > what Nitesh is implementing:
> > >
> > > 1) In arch_free_page, the being-freed page is added
> > > to a per-cpu set of freed pages.
> > > 2) Once that set is full, arch_free_pages goes into a
> > > slow path, which:
> > > 2a) Iterates over the set of freed pages, and
> > > 2b) Checks whether they are still free, and
> > > 2c) Adds the still free pages to a list that is
> > > to be passed to the hypervisor, to be MADV_FREEd.
> > > 2d) Makes that hypercall.
> > >
> > > Meanwhile all arch_alloc_pages has to do is make sure it
> > > does not allocate a page while it is currently being
> > > MADV_FREEd on the hypervisor side.
> > >
> > > The code Wei is working on looks like it could be
> > > suitable for steps (2c) and (2d) above. Nitesh already
> > > has code for steps 1 through 2b.
> >
> > So my question is this: Wei posted these numbers for balloon
> > inflation times:
> > inflating 7GB of an 8GB idle guest:
> >
> > 1) allocating pages (6.5%)
> > 2) sending PFNs to host (68.3%)
> > 3) address translation (6.1%)
> > 4) madvise (19%)
> >
> > It takes about 4126ms for the inflating process to complete.
> >
> > It seems that this is an excessive amount of time to stay
> > under a lock. What are your estimates for Nitesh's work?
>
> That depends on the batch size used for step
> (2c), and is something that we should be able
> to tune for decent performance.
I am not really sure how you intend to do this. Who will
drop and retake the lock? How do you make progress
instead of restarting from the beginning?
How do you combine multiple pages in a single s/g?
All these were issues that Wei's patches solved,
granted in a very limited manner (migration-specific)
but OTOH without a lot of tuning.
> What seems to matter is that things are batched.
> There are many ways to achieve that.
True, this is what the patches are trying to achieve. So far this
approach was the 1st more or less workable way do achieve that,
previous ones got us nowhere.
> --
> All rights reversed
^ permalink raw reply
* Re: [PATCH v11 4/6] mm: function to offer a page block on the free list
From: Christian Borntraeger @ 2017-06-21 12:56 UTC (permalink / raw)
To: David Hildenbrand, Rik van Riel, Dave Hansen, Wei Wang,
linux-kernel, qemu-devel, virtualization, kvm, linux-mm, mst,
cornelia.huck, akpm, mgorman, aarcange, amit.shah, pbonzini,
liliang.opensource
Cc: Nitesh Narayan Lal
In-Reply-To: <7b626551-6d1b-c8d5-4ef7-e357399e78dc@redhat.com>
On 06/20/2017 06:49 PM, David Hildenbrand wrote:
> On 20.06.2017 18:44, Rik van Riel wrote:
>> On Mon, 2017-06-12 at 07:10 -0700, Dave Hansen wrote:
>>
>>> The hypervisor is going to throw away the contents of these pages,
>>> right? As soon as the spinlock is released, someone can allocate a
>>> page, and put good data in it. What keeps the hypervisor from
>>> throwing
>>> away good data?
>>
>> That looks like it may be the wrong API, then?
>>
>> We already have hooks called arch_free_page and
>> arch_alloc_page in the VM, which are called when
>> pages are freed, and allocated, respectively.
>>
>> Nitesh Lal (on the CC list) is working on a way
>> to efficiently batch recently freed pages for
>> free page hinting to the hypervisor.
>>
>> If that is done efficiently enough (eg. with
>> MADV_FREE on the hypervisor side for lazy freeing,
>> and lazy later re-use of the pages), do we still
>> need the harder to use batch interface from this
>> patch?
>>
> David's opinion incoming:
>
> No, I think proper free page hinting would be the optimum solution, if
> done right. This would avoid the batch interface and even turn
> virtio-balloon in some sense useless.
>
Two reasons why I disagree:
- virtio-balloon is often used as memory hotplug. (e.g. libvirts current/max memory
uses virtio ballon)
- free page hinting will not allow to shrink the page cache of guests (like a ballooner does)
^ permalink raw reply
* Re: [PATCH v11 4/6] mm: function to offer a page block on the free list
From: David Hildenbrand @ 2017-06-21 13:47 UTC (permalink / raw)
To: Christian Borntraeger, Rik van Riel, Dave Hansen, Wei Wang,
linux-kernel, qemu-devel, virtualization, kvm, linux-mm, mst,
cornelia.huck, akpm, mgorman, aarcange, amit.shah, pbonzini,
liliang.opensource
Cc: Nitesh Narayan Lal
In-Reply-To: <c5f8cf53-c30b-a7ec-a8e8-9a2c120bdff6@de.ibm.com>
On 21.06.2017 14:56, Christian Borntraeger wrote:
> On 06/20/2017 06:49 PM, David Hildenbrand wrote:
>> On 20.06.2017 18:44, Rik van Riel wrote:
>>> On Mon, 2017-06-12 at 07:10 -0700, Dave Hansen wrote:
>>>
>>>> The hypervisor is going to throw away the contents of these pages,
>>>> right? As soon as the spinlock is released, someone can allocate a
>>>> page, and put good data in it. What keeps the hypervisor from
>>>> throwing
>>>> away good data?
>>>
>>> That looks like it may be the wrong API, then?
>>>
>>> We already have hooks called arch_free_page and
>>> arch_alloc_page in the VM, which are called when
>>> pages are freed, and allocated, respectively.
>>>
>>> Nitesh Lal (on the CC list) is working on a way
>>> to efficiently batch recently freed pages for
>>> free page hinting to the hypervisor.
>>>
>>> If that is done efficiently enough (eg. with
>>> MADV_FREE on the hypervisor side for lazy freeing,
>>> and lazy later re-use of the pages), do we still
>>> need the harder to use batch interface from this
>>> patch?
>>>
>> David's opinion incoming:
>>
>> No, I think proper free page hinting would be the optimum solution, if
>> done right. This would avoid the batch interface and even turn
>> virtio-balloon in some sense useless.
>>
I said "some sense" for a reason. Mainly because other techniques are
being worked on that are to fill the holes.
> Two reasons why I disagree:
> - virtio-balloon is often used as memory hotplug. (e.g. libvirts current/max memory
> uses virtio ballon)
I know, while one can argue if this real unplug as there are basically
no guarantees (see virtio-mem RFC) it is used by people because there is
simply no alternative. Still, for now some people use it for that.
> - free page hinting will not allow to shrink the page cache of guests (like a ballooner does)
There are currently some projects ongoing that try to avoid the page
cache in the guest completely.
--
Thanks,
David
^ permalink raw reply
* [PATCH] virtio_scsi: let host do exception handling
From: Paolo Bonzini @ 2017-06-21 14:35 UTC (permalink / raw)
To: linux-kernel, virtualization
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, stable,
Douglas Miller
virtio_scsi tries to do exception handling after the default
30 seconds timeout expires. However, it's better to let the host
control the timeout, otherwise with a heavy I/O load it is
likely that an abort will also timeout. This leads to fatal
errors like filesystems going offline.
Disable the 'sd' timeout and allow the host to do exception
handling, following the precedent of the storvsc driver.
Hannes has a proposal to introduce timeouts in virtio, but
this provides an immediate solution for stable kernels too.
Reported-by: Douglas Miller <dougmill@linux.vnet.ibm.com>
Cc: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: linux-scsi@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
drivers/scsi/virtio_scsi.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index f8dbfeee6c63..55d344ea6869 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -796,6 +796,16 @@ static int virtscsi_map_queues(struct Scsi_Host *shost)
return blk_mq_virtio_map_queues(&shost->tag_set, vscsi->vdev, 2);
}
+/*
+ * The host guarantees to respond to each command, although I/O latencies might
+ * be higher than on bare meta. Reset the timer unconditionally to give the
+ * host a chance to perform EH.
+ */
+static enum blk_eh_timer_return virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
+{
+ return BLK_EH_RESET_TIMER;
+}
+
static struct scsi_host_template virtscsi_host_template_single = {
.module = THIS_MODULE,
.name = "Virtio SCSI HBA",
@@ -806,6 +816,7 @@ static struct scsi_host_template virtscsi_host_template_single = {
.change_queue_depth = virtscsi_change_queue_depth,
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
+ .eh_timed_out = virtscsi_eh_timed_out,
.slave_alloc = virtscsi_device_alloc,
.can_queue = 1024,
@@ -826,6 +837,7 @@ static struct scsi_host_template virtscsi_host_template_multi = {
.change_queue_depth = virtscsi_change_queue_depth,
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
+ .eh_timed_out = virtscsi_eh_timed_out,
.can_queue = 1024,
.dma_boundary = UINT_MAX,
--
2.13.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox