* [PATCH 2/8] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
@ 2013-02-06 11:10 ` Chris Wilson
2013-02-07 5:09 ` Ben Widawsky
2013-02-06 11:10 ` [PATCH 3/8] drm/i915: Split the framebuffer_info creation into a separate routine Chris Wilson
` (8 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2013-02-06 11:10 UTC (permalink / raw)
To: intel-gfx
Wrap a preallocated region of stolen memory within an ordinary GEM
object, for example the BIOS framebuffer.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.h | 5 +++
drivers/gpu/drm/i915/i915_gem_stolen.c | 65 ++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 08c5def..fd8a495 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1720,6 +1720,11 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
void i915_gem_cleanup_stolen(struct drm_device *dev);
struct drm_i915_gem_object *
i915_gem_object_create_stolen(struct drm_device *dev, u32 size);
+struct drm_i915_gem_object *
+i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
+ u32 stolen_offset,
+ u32 gtt_offset,
+ u32 size);
void i915_gem_object_release_stolen(struct drm_i915_gem_object *obj);
/* i915_gem_tiling.c */
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 69d97cb..130d1db 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -312,6 +312,71 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
return NULL;
}
+struct drm_i915_gem_object *
+i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
+ u32 stolen_offset,
+ u32 gtt_offset,
+ u32 size)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_i915_gem_object *obj;
+ struct drm_mm_node *stolen;
+
+ if (dev_priv->mm.stolen_base == 0)
+ return NULL;
+
+ DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%x, gtt_offset=%x, size=%x\n",
+ stolen_offset, gtt_offset, size);
+
+ /* KISS and expect everything to be page-aligned */
+ BUG_ON(stolen_offset & 4095);
+ BUG_ON(gtt_offset & 4095);
+ BUG_ON(size & 4095);
+
+ if (WARN_ON(size == 0))
+ return NULL;
+
+ stolen = drm_mm_create_block(&dev_priv->mm.stolen,
+ stolen_offset, size,
+ false);
+ if (stolen == NULL) {
+ DRM_DEBUG_KMS("failed to allocate stolen space\n");
+ return NULL;
+ }
+
+ obj = _i915_gem_object_create_stolen(dev, stolen);
+ if (obj == NULL) {
+ DRM_DEBUG_KMS("failed to allocate stolen object\n");
+ drm_mm_put_block(stolen);
+ return NULL;
+ }
+
+ /* To simplify the initialisation sequence between KMS and GTT,
+ * we allow construction of the stolen object prior to
+ * setting up the GTT space. The actual reservation will occur
+ * later.
+ */
+ if (drm_mm_initialized(&dev_priv->mm.gtt_space)) {
+ obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
+ gtt_offset, size,
+ false);
+ if (obj->gtt_space == NULL) {
+ DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
+ drm_gem_object_unreference(&obj->base);
+ return NULL;
+ }
+ } else
+ obj->gtt_space = I915_GTT_RESERVED;
+
+ obj->gtt_offset = gtt_offset;
+ obj->has_global_gtt_mapping = 1;
+
+ list_add_tail(&obj->gtt_list, &dev_priv->mm.bound_list);
+ list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
+
+ return obj;
+}
+
void
i915_gem_object_release_stolen(struct drm_i915_gem_object *obj)
{
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 2/8] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated
2013-02-06 11:10 ` [PATCH 2/8] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated Chris Wilson
@ 2013-02-07 5:09 ` Ben Widawsky
2013-02-07 14:32 ` Chris Wilson
0 siblings, 1 reply; 20+ messages in thread
From: Ben Widawsky @ 2013-02-07 5:09 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, Feb 06, 2013 at 11:10:22AM +0000, Chris Wilson wrote:
> Wrap a preallocated region of stolen memory within an ordinary GEM
> object, for example the BIOS framebuffer.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 +++
> drivers/gpu/drm/i915/i915_gem_stolen.c | 65 ++++++++++++++++++++++++++++++++
> 2 files changed, 70 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 08c5def..fd8a495 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1720,6 +1720,11 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
> void i915_gem_cleanup_stolen(struct drm_device *dev);
> struct drm_i915_gem_object *
> i915_gem_object_create_stolen(struct drm_device *dev, u32 size);
> +struct drm_i915_gem_object *
> +i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
> + u32 stolen_offset,
> + u32 gtt_offset,
> + u32 size);
Can we default to u64 for things from now on. Not that I know anything,
or anything. At least gtt_offset.
> void i915_gem_object_release_stolen(struct drm_i915_gem_object *obj);
>
> /* i915_gem_tiling.c */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 69d97cb..130d1db 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -312,6 +312,71 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
> return NULL;
> }
>
> +struct drm_i915_gem_object *
> +i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
> + u32 stolen_offset,
> + u32 gtt_offset,
> + u32 size)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_gem_object *obj;
> + struct drm_mm_node *stolen;
> +
> + if (dev_priv->mm.stolen_base == 0)
> + return NULL;
> +
> + DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%x, gtt_offset=%x, size=%x\n",
> + stolen_offset, gtt_offset, size);
> +
> + /* KISS and expect everything to be page-aligned */
> + BUG_ON(stolen_offset & 4095);
> + BUG_ON(gtt_offset & 4095);
> + BUG_ON(size & 4095);
> +
> + if (WARN_ON(size == 0))
> + return NULL;
> +
> + stolen = drm_mm_create_block(&dev_priv->mm.stolen,
> + stolen_offset, size,
> + false);
> + if (stolen == NULL) {
> + DRM_DEBUG_KMS("failed to allocate stolen space\n");
> + return NULL;
> + }
> +
> + obj = _i915_gem_object_create_stolen(dev, stolen);
> + if (obj == NULL) {
> + DRM_DEBUG_KMS("failed to allocate stolen object\n");
> + drm_mm_put_block(stolen);
> + return NULL;
> + }
> +
> + /* To simplify the initialisation sequence between KMS and GTT,
> + * we allow construction of the stolen object prior to
> + * setting up the GTT space. The actual reservation will occur
> + * later.
> + */
> + if (drm_mm_initialized(&dev_priv->mm.gtt_space)) {
> + obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
> + gtt_offset, size,
> + false);
> + if (obj->gtt_space == NULL) {
> + DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
> + drm_gem_object_unreference(&obj->base);
> + return NULL;
> + }
> + } else
> + obj->gtt_space = I915_GTT_RESERVED;
Could you explain how/why we'd do this before the mm is initialized.
> +
> + obj->gtt_offset = gtt_offset;
> + obj->has_global_gtt_mapping = 1;
> +
> + list_add_tail(&obj->gtt_list, &dev_priv->mm.bound_list);
> + list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
> +
> + return obj;
> +}
> +
> void
> i915_gem_object_release_stolen(struct drm_i915_gem_object *obj)
> {
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/8] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated
2013-02-07 5:09 ` Ben Widawsky
@ 2013-02-07 14:32 ` Chris Wilson
2013-02-07 18:13 ` Ben Widawsky
0 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2013-02-07 14:32 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Wed, Feb 06, 2013 at 09:09:34PM -0800, Ben Widawsky wrote:
> On Wed, Feb 06, 2013 at 11:10:22AM +0000, Chris Wilson wrote:
> > Wrap a preallocated region of stolen memory within an ordinary GEM
> > object, for example the BIOS framebuffer.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 5 +++
> > drivers/gpu/drm/i915/i915_gem_stolen.c | 65 ++++++++++++++++++++++++++++++++
> > 2 files changed, 70 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 08c5def..fd8a495 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1720,6 +1720,11 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
> > void i915_gem_cleanup_stolen(struct drm_device *dev);
> > struct drm_i915_gem_object *
> > i915_gem_object_create_stolen(struct drm_device *dev, u32 size);
> > +struct drm_i915_gem_object *
> > +i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
> > + u32 stolen_offset,
> > + u32 gtt_offset,
> > + u32 size);
>
> Can we default to u64 for things from now on. Not that I know anything,
> or anything. At least gtt_offset.
We could. Do we envison getting more than 4GiB of stolen memory? Do you
worry that the BIOS may not use a 1:1 mapping?
> > + /* To simplify the initialisation sequence between KMS and GTT,
> > + * we allow construction of the stolen object prior to
> > + * setting up the GTT space. The actual reservation will occur
> > + * later.
> > + */
> > + if (drm_mm_initialized(&dev_priv->mm.gtt_space)) {
> > + obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
> > + gtt_offset, size,
> > + false);
> > + if (obj->gtt_space == NULL) {
> > + DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
> > + drm_gem_object_unreference(&obj->base);
> > + return NULL;
> > + }
> > + } else
> > + obj->gtt_space = I915_GTT_RESERVED;
>
> Could you explain how/why we'd do this before the mm is initialized.
It's all due to having to wrap the BIOS structures prior to us
initialising the mm. If we don't, we overwrite those objects whilst
setting up the GTT. Note that the display is currently scaning out from
those objects...
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 2/8] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated
2013-02-07 14:32 ` Chris Wilson
@ 2013-02-07 18:13 ` Ben Widawsky
0 siblings, 0 replies; 20+ messages in thread
From: Ben Widawsky @ 2013-02-07 18:13 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Thu, Feb 07, 2013 at 02:32:58PM +0000, Chris Wilson wrote:
> On Wed, Feb 06, 2013 at 09:09:34PM -0800, Ben Widawsky wrote:
> > On Wed, Feb 06, 2013 at 11:10:22AM +0000, Chris Wilson wrote:
> > > Wrap a preallocated region of stolen memory within an ordinary GEM
> > > object, for example the BIOS framebuffer.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 5 +++
> > > drivers/gpu/drm/i915/i915_gem_stolen.c | 65 ++++++++++++++++++++++++++++++++
> > > 2 files changed, 70 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 08c5def..fd8a495 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1720,6 +1720,11 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
> > > void i915_gem_cleanup_stolen(struct drm_device *dev);
> > > struct drm_i915_gem_object *
> > > i915_gem_object_create_stolen(struct drm_device *dev, u32 size);
> > > +struct drm_i915_gem_object *
> > > +i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
> > > + u32 stolen_offset,
> > > + u32 gtt_offset,
> > > + u32 size);
> >
> > Can we default to u64 for things from now on. Not that I know anything,
> > or anything. At least gtt_offset.
>
> We could. Do we envison getting more than 4GiB of stolen memory? Do you
> worry that the BIOS may not use a 1:1 mapping?
I suppose we can always change it later actually, so nevermind. I
wouldn't have envisioned the stolen size would grow as large as it has
today, so I am not the right person to ask anyway.
>
> > > + /* To simplify the initialisation sequence between KMS and GTT,
> > > + * we allow construction of the stolen object prior to
> > > + * setting up the GTT space. The actual reservation will occur
> > > + * later.
> > > + */
> > > + if (drm_mm_initialized(&dev_priv->mm.gtt_space)) {
> > > + obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
> > > + gtt_offset, size,
> > > + false);
> > > + if (obj->gtt_space == NULL) {
> > > + DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
> > > + drm_gem_object_unreference(&obj->base);
> > > + return NULL;
> > > + }
> > > + } else
> > > + obj->gtt_space = I915_GTT_RESERVED;
> >
> > Could you explain how/why we'd do this before the mm is initialized.
>
> It's all due to having to wrap the BIOS structures prior to us
> initialising the mm. If we don't, we overwrite those objects whilst
> setting up the GTT. Note that the display is currently scaning out from
> those objects...
> -Chris
For some reason, I thought the code that had already landed took care of
this by not initializing those PTEs. My bad.
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/8] drm/i915: Split the framebuffer_info creation into a separate routine
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
2013-02-06 11:10 ` [PATCH 2/8] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated Chris Wilson
@ 2013-02-06 11:10 ` Chris Wilson
2013-03-07 14:49 ` Imre Deak
2013-02-06 11:10 ` [PATCH 4/8] drm: add initial_config function to fb helper Chris Wilson
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2013-02-06 11:10 UTC (permalink / raw)
To: intel-gfx
This will be shared with wrapping the BIOS framebuffer into the fbdev
later. In the meantime, we can tidy the code slightly and improve the
error path handling.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_display.c | 7 --
drivers/gpu/drm/i915/intel_drv.h | 7 ++
drivers/gpu/drm/i915/intel_fb.c | 154 ++++++++++++++++++----------------
3 files changed, 91 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f1dbd01..8f9cdd7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6413,13 +6413,6 @@ intel_framebuffer_create(struct drm_device *dev,
}
static u32
-intel_framebuffer_pitch_for_width(int width, int bpp)
-{
- u32 pitch = DIV_ROUND_UP(width * bpp, 8);
- return ALIGN(pitch, 64);
-}
-
-static u32
intel_framebuffer_size_for_mode(struct drm_display_mode *mode, int bpp)
{
u32 pitch = intel_framebuffer_pitch_for_width(mode->hdisplay, bpp);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 13afb37..07d95a1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -134,6 +134,13 @@ struct intel_framebuffer {
struct drm_i915_gem_object *obj;
};
+inline static u32
+intel_framebuffer_pitch_for_width(int width, int bpp)
+{
+ u32 pitch = DIV_ROUND_UP(width * bpp, 8);
+ return ALIGN(pitch, 64);
+}
+
struct intel_fbdev {
struct drm_fb_helper helper;
struct intel_framebuffer ifb;
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 6591029..de0ac4c 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -57,29 +57,96 @@ static struct fb_ops intelfb_ops = {
.fb_debug_leave = drm_fb_helper_debug_leave,
};
+static struct fb_info *intelfb_create_info(struct intel_fbdev *ifbdev)
+{
+ struct drm_framebuffer *fb = &ifbdev->ifb.base;
+ struct drm_device *dev = fb->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct fb_info *info;
+ u32 gtt_offset, size;
+ int ret;
+
+ info = framebuffer_alloc(0, &dev->pdev->dev);
+ if (!info)
+ return NULL;
+
+ info->par = ifbdev;
+ ifbdev->helper.fb = fb;
+ ifbdev->helper.fbdev = info;
+
+ strcpy(info->fix.id, "inteldrmfb");
+
+ info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
+ info->fbops = &intelfb_ops;
+
+ ret = fb_alloc_cmap(&info->cmap, 256, 0);
+ if (ret)
+ goto err_info;
+
+ /* setup aperture base/size for vesafb takeover */
+ info->apertures = alloc_apertures(1);
+ if (!info->apertures)
+ goto err_cmap;
+
+ info->apertures->ranges[0].base = dev->mode_config.fb_base;
+ info->apertures->ranges[0].size = dev_priv->gtt.mappable_end;
+
+ gtt_offset = ifbdev->ifb.obj->gtt_offset;
+ size = ifbdev->ifb.obj->base.size;
+
+ info->fix.smem_start = dev->mode_config.fb_base + gtt_offset;
+ info->fix.smem_len = size;
+
+ info->screen_size = size;
+ info->screen_base = ioremap_wc(dev_priv->gtt.mappable_base + gtt_offset,
+ size);
+ if (!info->screen_base)
+ goto err_cmap;
+
+ /* If the object is shmemfs backed, it will have given us zeroed pages.
+ * If the object is stolen however, it will be full of whatever
+ * garbage was left in there.
+ */
+ if (ifbdev->ifb.obj->stolen)
+ memset_io(info->screen_base, 0, info->screen_size);
+
+ /* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
+
+ drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
+ drm_fb_helper_fill_var(info, &ifbdev->helper, fb->width, fb->height);
+
+ return info;
+
+err_cmap:
+ if (info->cmap.len)
+ fb_dealloc_cmap(&info->cmap);
+err_info:
+ framebuffer_release(info);
+ return NULL;
+}
+
static int intelfb_create(struct intel_fbdev *ifbdev,
struct drm_fb_helper_surface_size *sizes)
{
struct drm_device *dev = ifbdev->helper.dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct fb_info *info;
- struct drm_framebuffer *fb;
- struct drm_mode_fb_cmd2 mode_cmd = {};
+ struct drm_mode_fb_cmd2 mode_cmd = { 0 };
struct drm_i915_gem_object *obj;
- struct device *device = &dev->pdev->dev;
+ struct fb_info *info;
int size, ret;
/* we don't do packed 24bpp */
if (sizes->surface_bpp == 24)
sizes->surface_bpp = 32;
- mode_cmd.width = sizes->surface_width;
+ mode_cmd.width = sizes->surface_width;
mode_cmd.height = sizes->surface_height;
- mode_cmd.pitches[0] = ALIGN(mode_cmd.width * ((sizes->surface_bpp + 7) /
- 8), 64);
- mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
- sizes->surface_depth);
+ mode_cmd.pitches[0] =
+ intel_framebuffer_pitch_for_width(mode_cmd.width,
+ sizes->surface_bpp);
+ mode_cmd.pixel_format =
+ drm_mode_legacy_fb_format(sizes->surface_bpp,
+ sizes->surface_depth);
size = mode_cmd.pitches[0] * mode_cmd.height;
size = ALIGN(size, PAGE_SIZE);
@@ -101,72 +168,19 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
goto out_unref;
}
- info = framebuffer_alloc(0, device);
- if (!info) {
- ret = -ENOMEM;
- goto out_unpin;
- }
-
- info->par = ifbdev;
-
ret = intel_framebuffer_init(dev, &ifbdev->ifb, &mode_cmd, obj);
if (ret)
goto out_unpin;
- fb = &ifbdev->ifb.base;
-
- ifbdev->helper.fb = fb;
- ifbdev->helper.fbdev = info;
-
- strcpy(info->fix.id, "inteldrmfb");
-
- info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
- info->fbops = &intelfb_ops;
+ DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08x, bo %p\n",
+ mode_cmd.width, mode_cmd.height,
+ obj->gtt_offset, obj);
- ret = fb_alloc_cmap(&info->cmap, 256, 0);
- if (ret) {
- ret = -ENOMEM;
- goto out_unpin;
- }
- /* setup aperture base/size for vesafb takeover */
- info->apertures = alloc_apertures(1);
- if (!info->apertures) {
+ info = intelfb_create_info(ifbdev);
+ if (info == NULL) {
ret = -ENOMEM;
goto out_unpin;
}
- info->apertures->ranges[0].base = dev->mode_config.fb_base;
- info->apertures->ranges[0].size = dev_priv->gtt.mappable_end;
-
- info->fix.smem_start = dev->mode_config.fb_base + obj->gtt_offset;
- info->fix.smem_len = size;
-
- info->screen_base =
- ioremap_wc(dev_priv->gtt.mappable_base + obj->gtt_offset,
- size);
- if (!info->screen_base) {
- ret = -ENOSPC;
- goto out_unpin;
- }
- info->screen_size = size;
-
-// memset(info->screen_base, 0, size);
-
- drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
- drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);
-
- /* If the object is shmemfs backed, it will have given us zeroed pages.
- * If the object is stolen however, it will be full of whatever
- * garbage was left in there.
- */
- if (ifbdev->ifb.obj->stolen)
- memset_io(info->screen_base, 0, info->screen_size);
-
- /* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
-
- DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08x, bo %p\n",
- fb->width, fb->height,
- obj->gtt_offset, obj);
-
mutex_unlock(&dev->struct_mutex);
vga_switcheroo_client_fb_set(dev->pdev, info);
@@ -206,11 +220,11 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
static void intel_fbdev_destroy(struct drm_device *dev,
struct intel_fbdev *ifbdev)
{
- struct fb_info *info;
struct intel_framebuffer *ifb = &ifbdev->ifb;
if (ifbdev->helper.fbdev) {
- info = ifbdev->helper.fbdev;
+ struct fb_info *info = ifbdev->helper.fbdev;
+
unregister_framebuffer(info);
iounmap(info->screen_base);
if (info->cmap.len)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 3/8] drm/i915: Split the framebuffer_info creation into a separate routine
2013-02-06 11:10 ` [PATCH 3/8] drm/i915: Split the framebuffer_info creation into a separate routine Chris Wilson
@ 2013-03-07 14:49 ` Imre Deak
0 siblings, 0 replies; 20+ messages in thread
From: Imre Deak @ 2013-03-07 14:49 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, 2013-02-06 at 11:10 +0000, Chris Wilson wrote:
> This will be shared with wrapping the BIOS framebuffer into the fbdev
> later. In the meantime, we can tidy the code slightly and improve the
> error path handling.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_display.c | 7 --
> drivers/gpu/drm/i915/intel_drv.h | 7 ++
> drivers/gpu/drm/i915/intel_fb.c | 154 ++++++++++++++++++----------------
> 3 files changed, 91 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f1dbd01..8f9cdd7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6413,13 +6413,6 @@ intel_framebuffer_create(struct drm_device *dev,
> }
>
> static u32
> -intel_framebuffer_pitch_for_width(int width, int bpp)
> -{
> - u32 pitch = DIV_ROUND_UP(width * bpp, 8);
> - return ALIGN(pitch, 64);
> -}
> -
> -static u32
> intel_framebuffer_size_for_mode(struct drm_display_mode *mode, int bpp)
> {
> u32 pitch = intel_framebuffer_pitch_for_width(mode->hdisplay, bpp);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 13afb37..07d95a1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -134,6 +134,13 @@ struct intel_framebuffer {
> struct drm_i915_gem_object *obj;
> };
>
> +inline static u32
> +intel_framebuffer_pitch_for_width(int width, int bpp)
> +{
> + u32 pitch = DIV_ROUND_UP(width * bpp, 8);
> + return ALIGN(pitch, 64);
> +}
> +
> struct intel_fbdev {
> struct drm_fb_helper helper;
> struct intel_framebuffer ifb;
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index 6591029..de0ac4c 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -57,29 +57,96 @@ static struct fb_ops intelfb_ops = {
> .fb_debug_leave = drm_fb_helper_debug_leave,
> };
>
> +static struct fb_info *intelfb_create_info(struct intel_fbdev *ifbdev)
> +{
> + struct drm_framebuffer *fb = &ifbdev->ifb.base;
> + struct drm_device *dev = fb->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct fb_info *info;
> + u32 gtt_offset, size;
> + int ret;
> +
> + info = framebuffer_alloc(0, &dev->pdev->dev);
> + if (!info)
> + return NULL;
> +
> + info->par = ifbdev;
> + ifbdev->helper.fb = fb;
> + ifbdev->helper.fbdev = info;
> +
> + strcpy(info->fix.id, "inteldrmfb");
> +
> + info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
> + info->fbops = &intelfb_ops;
> +
> + ret = fb_alloc_cmap(&info->cmap, 256, 0);
> + if (ret)
> + goto err_info;
> +
> + /* setup aperture base/size for vesafb takeover */
> + info->apertures = alloc_apertures(1);
> + if (!info->apertures)
> + goto err_cmap;
> +
> + info->apertures->ranges[0].base = dev->mode_config.fb_base;
> + info->apertures->ranges[0].size = dev_priv->gtt.mappable_end;
> +
> + gtt_offset = ifbdev->ifb.obj->gtt_offset;
> + size = ifbdev->ifb.obj->base.size;
> +
> + info->fix.smem_start = dev->mode_config.fb_base + gtt_offset;
> + info->fix.smem_len = size;
> +
> + info->screen_size = size;
> + info->screen_base = ioremap_wc(dev_priv->gtt.mappable_base + gtt_offset,
> + size);
> + if (!info->screen_base)
> + goto err_cmap;
kfree(info->apertures) is missing. Same in intel_fbdev_destroy().
> +
> + /* If the object is shmemfs backed, it will have given us zeroed pages.
> + * If the object is stolen however, it will be full of whatever
> + * garbage was left in there.
> + */
> + if (ifbdev->ifb.obj->stolen)
> + memset_io(info->screen_base, 0, info->screen_size);
> +
> + /* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
> +
> + drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
> + drm_fb_helper_fill_var(info, &ifbdev->helper, fb->width, fb->height);
> +
> + return info;
> +
> +err_cmap:
> + if (info->cmap.len)
> + fb_dealloc_cmap(&info->cmap);
fb_dealloc_cmap() could be called unconditionally.
> +err_info:
> + framebuffer_release(info);
> + return NULL;
> +}
> +
> static int intelfb_create(struct intel_fbdev *ifbdev,
> struct drm_fb_helper_surface_size *sizes)
> {
> struct drm_device *dev = ifbdev->helper.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct fb_info *info;
> - struct drm_framebuffer *fb;
> - struct drm_mode_fb_cmd2 mode_cmd = {};
> + struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> struct drm_i915_gem_object *obj;
> - struct device *device = &dev->pdev->dev;
> + struct fb_info *info;
> int size, ret;
>
> /* we don't do packed 24bpp */
> if (sizes->surface_bpp == 24)
> sizes->surface_bpp = 32;
>
> - mode_cmd.width = sizes->surface_width;
> + mode_cmd.width = sizes->surface_width;
> mode_cmd.height = sizes->surface_height;
>
> - mode_cmd.pitches[0] = ALIGN(mode_cmd.width * ((sizes->surface_bpp + 7) /
> - 8), 64);
> - mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> - sizes->surface_depth);
> + mode_cmd.pitches[0] =
> + intel_framebuffer_pitch_for_width(mode_cmd.width,
> + sizes->surface_bpp);
This changes pitches[0] for non 8-bit aligned bpps. If it's a fix a
comment about it in the commit message would be nice.
--Imre
> + mode_cmd.pixel_format =
> + drm_mode_legacy_fb_format(sizes->surface_bpp,
> + sizes->surface_depth);
>
> size = mode_cmd.pitches[0] * mode_cmd.height;
> size = ALIGN(size, PAGE_SIZE);
> @@ -101,72 +168,19 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
> goto out_unref;
> }
>
> - info = framebuffer_alloc(0, device);
> - if (!info) {
> - ret = -ENOMEM;
> - goto out_unpin;
> - }
> -
> - info->par = ifbdev;
> -
> ret = intel_framebuffer_init(dev, &ifbdev->ifb, &mode_cmd, obj);
> if (ret)
> goto out_unpin;
>
> - fb = &ifbdev->ifb.base;
> -
> - ifbdev->helper.fb = fb;
> - ifbdev->helper.fbdev = info;
> -
> - strcpy(info->fix.id, "inteldrmfb");
> -
> - info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
> - info->fbops = &intelfb_ops;
> + DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08x, bo %p\n",
> + mode_cmd.width, mode_cmd.height,
> + obj->gtt_offset, obj);
>
> - ret = fb_alloc_cmap(&info->cmap, 256, 0);
> - if (ret) {
> - ret = -ENOMEM;
> - goto out_unpin;
> - }
> - /* setup aperture base/size for vesafb takeover */
> - info->apertures = alloc_apertures(1);
> - if (!info->apertures) {
> + info = intelfb_create_info(ifbdev);
> + if (info == NULL) {
> ret = -ENOMEM;
> goto out_unpin;
> }
> - info->apertures->ranges[0].base = dev->mode_config.fb_base;
> - info->apertures->ranges[0].size = dev_priv->gtt.mappable_end;
> -
> - info->fix.smem_start = dev->mode_config.fb_base + obj->gtt_offset;
> - info->fix.smem_len = size;
> -
> - info->screen_base =
> - ioremap_wc(dev_priv->gtt.mappable_base + obj->gtt_offset,
> - size);
> - if (!info->screen_base) {
> - ret = -ENOSPC;
> - goto out_unpin;
> - }
> - info->screen_size = size;
> -
> -// memset(info->screen_base, 0, size);
> -
> - drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
> - drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);
> -
> - /* If the object is shmemfs backed, it will have given us zeroed pages.
> - * If the object is stolen however, it will be full of whatever
> - * garbage was left in there.
> - */
> - if (ifbdev->ifb.obj->stolen)
> - memset_io(info->screen_base, 0, info->screen_size);
> -
> - /* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
> -
> - DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08x, bo %p\n",
> - fb->width, fb->height,
> - obj->gtt_offset, obj);
> -
>
> mutex_unlock(&dev->struct_mutex);
> vga_switcheroo_client_fb_set(dev->pdev, info);
> @@ -206,11 +220,11 @@ static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
> static void intel_fbdev_destroy(struct drm_device *dev,
> struct intel_fbdev *ifbdev)
> {
> - struct fb_info *info;
> struct intel_framebuffer *ifb = &ifbdev->ifb;
>
> if (ifbdev->helper.fbdev) {
> - info = ifbdev->helper.fbdev;
> + struct fb_info *info = ifbdev->helper.fbdev;
> +
> unregister_framebuffer(info);
> iounmap(info->screen_base);
> if (info->cmap.len)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/8] drm: add initial_config function to fb helper
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
2013-02-06 11:10 ` [PATCH 2/8] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated Chris Wilson
2013-02-06 11:10 ` [PATCH 3/8] drm/i915: Split the framebuffer_info creation into a separate routine Chris Wilson
@ 2013-02-06 11:10 ` Chris Wilson
2013-02-06 11:10 ` [PATCH 5/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon Chris Wilson
` (6 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2013-02-06 11:10 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
From: Jesse Barnes <jbarnes@virtuousgeek.org>
Rather than building a config which may or may not work, let the driver
build an initial fb config. This allows the driver to use the BIOS boot
configuration for example, displaying kernel messages and the initial fb
console on the same outputs the BIOS lit up at boot time. If that
fails, the driver can still fall back the same way as the core.
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/drm_fb_helper.c | 23 +++++++++++++++--------
include/drm/drm_fb_helper.h | 4 ++++
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 954d175..924901d 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1247,7 +1247,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
struct drm_mode_set *modeset;
bool *enabled;
int width, height;
- int i, ret;
+ int i;
DRM_DEBUG_KMS("\n");
@@ -1268,16 +1268,23 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
drm_enable_connectors(fb_helper, enabled);
- ret = drm_target_cloned(fb_helper, modes, enabled, width, height);
- if (!ret) {
- ret = drm_target_preferred(fb_helper, modes, enabled, width, height);
- if (!ret)
+ if (!(fb_helper->funcs->initial_config &&
+ fb_helper->funcs->initial_config(fb_helper, crtcs, modes,
+ enabled, width, height))) {
+ memset(modes, 0, dev->mode_config.num_connector*sizeof(modes[0]));
+ memset(crtcs, 0, dev->mode_config.num_connector*sizeof(crtcs[0]));
+
+ if (!drm_target_cloned(fb_helper,
+ modes, enabled, width, height) &&
+ !drm_target_preferred(fb_helper,
+ modes, enabled, width, height))
DRM_ERROR("Unable to find initial modes\n");
- }
- DRM_DEBUG_KMS("picking CRTCs for %dx%d config\n", width, height);
+ DRM_DEBUG_KMS("picking CRTCs for %dx%d config\n",
+ width, height);
- drm_pick_crtcs(fb_helper, crtcs, modes, 0, width, height);
+ drm_pick_crtcs(fb_helper, crtcs, modes, 0, width, height);
+ }
/* need to set the modesets up here for use later */
/* fill out the connector<->crtc mappings into the modesets */
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 5120b01..40b0c5c 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -56,6 +56,10 @@ struct drm_fb_helper_funcs {
int (*fb_probe)(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes);
+ bool (*initial_config)(struct drm_fb_helper *fb_helper,
+ struct drm_fb_helper_crtc **crtcs,
+ struct drm_display_mode **modes,
+ bool *enabled, int width, int height);
};
struct drm_fb_helper_connector {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 5/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
` (2 preceding siblings ...)
2013-02-06 11:10 ` [PATCH 4/8] drm: add initial_config function to fb helper Chris Wilson
@ 2013-02-06 11:10 ` Chris Wilson
2013-03-08 12:05 ` Imre Deak
2013-02-06 11:10 ` [PATCH 6/8] drm/i915: Retrieve the current mode upon KMS takeover Chris Wilson
` (5 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2013-02-06 11:10 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/i915_dma.c | 8 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_display.c | 14 +-
drivers/gpu/drm/i915/intel_drv.h | 4 +
drivers/gpu/drm/i915/intel_fb.c | 305 +++++++++++++++++++++++++++++++---
5 files changed, 306 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4fa6beb..f2b7db7 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1273,6 +1273,7 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
static int i915_load_modeset_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ bool was_vga_enabled;
int ret;
ret = intel_parse_bios(dev);
@@ -1309,7 +1310,11 @@ static int i915_load_modeset_init(struct drm_device *dev)
/* Important: The output setup functions called by modeset_init need
* working irqs for e.g. gmbus and dp aux transfers. */
- intel_modeset_init(dev);
+ intel_modeset_init(dev, &was_vga_enabled);
+
+ /* Wrap existing BIOS mode configuration prior to GEM takeover */
+ if (!was_vga_enabled)
+ intel_fbdev_init_bios(dev);
ret = i915_gem_init(dev);
if (ret)
@@ -1323,6 +1328,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
/* FIXME: do pre/post-mode set stuff in core KMS code */
dev->vblank_disable_allowed = 1;
+ /* Install a default KMS/GEM fbcon if we failed to wrap the BIOS fb */
ret = intel_fbdev_init(dev);
if (ret)
goto cleanup_gem;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fd8a495..6f4afbf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1814,7 +1814,7 @@ static inline void intel_unregister_dsm_handler(void) { return; }
/* modesetting */
extern void intel_modeset_init_hw(struct drm_device *dev);
-extern void intel_modeset_init(struct drm_device *dev);
+extern void intel_modeset_init(struct drm_device *dev, bool *was_vga_enabled);
extern void intel_modeset_gem_init(struct drm_device *dev);
extern void intel_modeset_cleanup(struct drm_device *dev);
extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8f9cdd7..7c4c7d5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8698,12 +8698,17 @@ static void intel_init_quirks(struct drm_device *dev)
}
/* Disable the VGA plane that we never use */
-static void i915_disable_vga(struct drm_device *dev)
+static bool i915_disable_vga(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ bool was_enabled;
u8 sr1;
u32 vga_reg = i915_vgacntrl_reg(dev);
+ was_enabled = !(I915_READ(vga_reg) & VGA_DISP_DISABLE);
+ DRM_DEBUG_KMS("VGA output is currently %s\n",
+ was_enabled ? "enabled" : "disabled");
+
vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
outb(SR01, VGA_SR_INDEX);
sr1 = inb(VGA_SR_DATA);
@@ -8713,6 +8718,8 @@ static void i915_disable_vga(struct drm_device *dev)
I915_WRITE(vga_reg, VGA_DISP_DISABLE);
POSTING_READ(vga_reg);
+
+ return was_enabled;
}
void intel_modeset_init_hw(struct drm_device *dev)
@@ -8728,7 +8735,8 @@ void intel_modeset_init_hw(struct drm_device *dev)
mutex_unlock(&dev->struct_mutex);
}
-void intel_modeset_init(struct drm_device *dev)
+void intel_modeset_init(struct drm_device *dev,
+ bool *was_vga_enabled)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int i, ret;
@@ -8775,7 +8783,7 @@ void intel_modeset_init(struct drm_device *dev)
intel_pch_pll_init(dev);
/* Just disable it once at startup */
- i915_disable_vga(dev);
+ *was_vga_enabled = i915_disable_vga(dev);
intel_setup_outputs(dev);
/* Just in case the BIOS is doing something questionable. */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 07d95a1..985e9dd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -146,6 +146,8 @@ struct intel_fbdev {
struct intel_framebuffer ifb;
struct list_head fbdev_list;
struct drm_display_mode *our_mode;
+ bool stolen;
+ int preferred_bpp;
};
struct intel_encoder {
@@ -212,6 +214,7 @@ struct intel_crtc {
enum plane plane;
enum transcoder cpu_transcoder;
u8 lut_r[256], lut_g[256], lut_b[256];
+ bool mode_valid;
/*
* Whether the crtc and the connected output pipeline is active. Implies
* that crtc->enabled is set, i.e. the current mode configuration has
@@ -612,6 +615,7 @@ extern int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *ifb,
struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_i915_gem_object *obj);
+extern void intel_fbdev_init_bios(struct drm_device *dev);
extern int intel_fbdev_init(struct drm_device *dev);
extern void intel_fbdev_initial_config(struct drm_device *dev);
extern void intel_fbdev_fini(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index de0ac4c..ca6c8e6 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -131,7 +131,6 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
struct drm_device *dev = ifbdev->helper.dev;
struct drm_mode_fb_cmd2 mode_cmd = { 0 };
struct drm_i915_gem_object *obj;
- struct fb_info *info;
int size, ret;
/* we don't do packed 24bpp */
@@ -176,14 +175,7 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
mode_cmd.width, mode_cmd.height,
obj->gtt_offset, obj);
- info = intelfb_create_info(ifbdev);
- if (info == NULL) {
- ret = -ENOMEM;
- goto out_unpin;
- }
-
mutex_unlock(&dev->struct_mutex);
- vga_switcheroo_client_fb_set(dev->pdev, info);
return 0;
out_unpin:
@@ -200,17 +192,92 @@ static int intel_fb_find_or_create_single(struct drm_fb_helper *helper,
{
struct intel_fbdev *ifbdev = (struct intel_fbdev *)helper;
int new_fb = 0;
- int ret;
if (!helper->fb) {
- ret = intelfb_create(ifbdev, sizes);
- if (ret)
- return ret;
+ struct fb_info *info;
+
+ if (!ifbdev->stolen) {
+ int ret = intelfb_create(ifbdev, sizes);
+ if (ret)
+ return ret;
+ }
+
+ info = intelfb_create_info(ifbdev);
+ if (info == NULL) {
+ DRM_DEBUG_KMS("fb creation failed\n");
+ return -ENOMEM;
+ }
+ vga_switcheroo_client_fb_set(helper->dev->pdev, info);
+
new_fb = 1;
}
+
return new_fb;
}
+static struct drm_fb_helper_crtc *
+intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
+{
+ int i;
+
+ for (i = 0; i < fb_helper->crtc_count; i++)
+ if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
+ return &fb_helper->crtc_info[i];
+
+ return NULL;
+}
+
+static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
+ struct drm_fb_helper_crtc **crtcs,
+ struct drm_display_mode **modes,
+ bool *enabled, int width, int height)
+{
+ int i;
+
+ for (i = 0; i < fb_helper->connector_count; i++) {
+ struct drm_connector *connector;
+ struct drm_encoder *encoder;
+
+ connector = fb_helper->connector_info[i]->connector;
+ if (!enabled[i]) {
+ DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
+ connector->base.id);
+ continue;
+ }
+
+ encoder = connector->encoder;
+ if (!encoder || !encoder->crtc) {
+ DRM_DEBUG_KMS("connector %d has no encoder or crtc, skipping\n",
+ connector->base.id);
+ continue;
+ }
+
+ if (WARN_ON(!encoder->crtc->enabled)) {
+ DRM_DEBUG_KMS("connector %s on crtc %d has inconsistent state, aborting\n",
+ drm_get_connector_name(connector),
+ encoder->crtc->base.id);
+ return false;
+ }
+
+ if (!to_intel_crtc(encoder->crtc)->mode_valid) {
+ DRM_DEBUG_KMS("connector %s on crtc %d has an invalid mode, aborting\n",
+ drm_get_connector_name(connector),
+ encoder->crtc->base.id);
+ return false;
+ }
+
+ modes[i] = &encoder->crtc->mode;
+ crtcs[i] = intel_fb_helper_crtc(fb_helper, encoder->crtc);
+
+ DRM_DEBUG_KMS("connector %s on crtc %d: %s\n",
+ drm_get_connector_name(connector),
+ encoder->crtc->base.id,
+ modes[i]->name);
+ }
+
+ return true;
+}
+
static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
.gamma_set = intel_crtc_fb_gamma_set,
.gamma_get = intel_crtc_fb_gamma_get,
@@ -241,23 +308,215 @@ static void intel_fbdev_destroy(struct drm_device *dev,
}
}
-int intel_fbdev_init(struct drm_device *dev)
+static bool pipe_enabled(struct drm_i915_private *dev_priv, enum pipe pipe)
{
+ enum transcoder cpu_transcoder =
+ intel_pipe_to_cpu_transcoder(dev_priv, pipe);
+ return !!(I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE);
+}
+
+/*
+ * Try to read the BIOS display configuration and use it for the initial
+ * fb configuration.
+ *
+ * The BIOS or boot loader will generally create an initial display
+ * configuration for us that includes some set of active pipes and displays.
+ * This routine tries to figure out which pipes are active, what resolutions
+ * are being displayed, and then allocates a framebuffer and initial fb
+ * config based on that data.
+ *
+ * If the BIOS or boot loader leaves the display in VGA mode, there's not
+ * much we can do; switching out of that mode involves allocating a new,
+ * high res buffer, and also recalculating bandwidth requirements for the
+ * new bpp configuration.
+ *
+ * However, if we're loaded into an existing, high res mode, we should
+ * be able to allocate a buffer big enough to handle the largest active
+ * mode, create a mode_set for it, and pass it to the fb helper to create
+ * the configuration.
+ */
+void intel_fbdev_init_bios(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_fbdev *ifbdev;
- drm_i915_private_t *dev_priv = dev->dev_private;
- int ret;
+ struct drm_crtc *crtc;
+ struct drm_mode_fb_cmd2 mode_cmd = { 0 };
+ struct drm_i915_gem_object *obj;
+ u32 obj_offset = 0;
+ int mode_bpp = 0;
+ u32 active = 0;
+
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ int pipe = intel_crtc->pipe, plane = intel_crtc->plane;
+ u32 val, bpp, offset, format;
+ int pitch, width, height;
+
+ if (!pipe_enabled(dev_priv, pipe)) {
+ DRM_DEBUG_KMS("pipe %c not active, skipping\n",
+ pipe_name(pipe));
+ continue;
+ }
+
+ val = I915_READ(DSPCNTR(plane));
+
+ if (INTEL_INFO(dev)->gen >= 4) {
+ if (val & DISPPLANE_TILED) {
+ DRM_DEBUG_KMS("tiled BIOS fb?\n");
+ continue; /* unexpected! */
+ }
+ }
+
+ switch (val & DISPPLANE_PIXFORMAT_MASK) {
+ case DISPPLANE_YUV422:
+ default:
+ DRM_DEBUG_KMS("pipe %c unsupported pixel format %x, skipping\n",
+ pipe_name(pipe), (val & DISPPLANE_PIXFORMAT_MASK) >> 26);
+ continue;
+ case DISPPLANE_8BPP:
+ format = DRM_FORMAT_C8;
+ bpp = 8;
+ break;
+ case DISPPLANE_BGRX555:
+ format = DRM_FORMAT_XRGB1555;
+ bpp = 16;
+ break;
+ case DISPPLANE_BGRX565:
+ format = DRM_FORMAT_RGB565;
+ bpp = 16;
+ break;
+ case DISPPLANE_BGRX888:
+ format = DRM_FORMAT_XRGB8888;
+ bpp = 32;
+ break;
+ }
+
+ if (mode_cmd.pixel_format == 0) {
+ mode_bpp = bpp;
+ mode_cmd.pixel_format = format;
+ }
+
+ if (mode_cmd.pixel_format != format) {
+ DRM_DEBUG_KMS("pipe %c has format/bpp (%d, %d) mismatch: skipping\n",
+ pipe_name(pipe), format, bpp);
+ continue;
+ }
+
+ if (INTEL_INFO(dev)->gen >= 4) {
+ if (I915_READ(DSPTILEOFF(plane))) {
+ DRM_DEBUG_KMS("pipe %c is offset: skipping\n",
+ pipe_name(pipe));
+ continue;
+ }
+
+ offset = I915_READ(DSPSURF(plane));
+ } else
+ offset = I915_READ(DSPADDR(plane));
+ if (!obj_offset)
+ obj_offset = offset;
+
+ pitch = I915_READ(DSPSTRIDE(plane));
+ if (mode_cmd.pitches[0] == 0)
+ mode_cmd.pitches[0] = pitch;
+
+ if (offset != obj_offset || pitch != mode_cmd.pitches[0]) {
+ DRM_DEBUG_KMS("multiple pipe setup not in clone mode, sjipping\n");
+ continue;
+ }
+
+ val = I915_READ(PIPESRC(pipe));
+ width = ((val >> 16) & 0xfff) + 1;
+ height = ((val >> 0) & 0xfff) + 1;
+
+ DRM_DEBUG_KMS("Found active pipe [%d/%d]: size=%dx%d@%d, offset=%x\n",
+ pipe, plane, width, height, bpp, offset);
+
+ if (width > mode_cmd.width)
+ mode_cmd.width = width;
+
+ if (height > mode_cmd.height)
+ mode_cmd.height = height;
+
+ active |= 1 << pipe;
+ }
+
+ if (active == 0) {
+ DRM_DEBUG_KMS("no active pipes found, not using BIOS config\n");
+ return;
+ }
ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
- if (!ifbdev)
- return -ENOMEM;
+ if (ifbdev == NULL) {
+ DRM_DEBUG_KMS("failed to alloc intel fbdev\n");
+ return;
+ }
- dev_priv->fbdev = ifbdev;
+ ifbdev->stolen = true;
+ ifbdev->preferred_bpp = mode_bpp;
ifbdev->helper.funcs = &intel_fb_helper_funcs;
+ ifbdev->helper.funcs->initial_config = intel_fb_initial_config;
+
+ /* assume a 1:1 linear mapping between stolen and GTT */
+ obj = i915_gem_object_create_stolen_for_preallocated(dev,
+ obj_offset,
+ obj_offset,
+ ALIGN(mode_cmd.pitches[0] * mode_cmd.height, PAGE_SIZE));
+ if (obj == NULL) {
+ DRM_DEBUG_KMS("failed to create stolen fb\n");
+ goto out_free_ifbdev;
+ }
+
+ if (intel_framebuffer_init(dev, &ifbdev->ifb, &mode_cmd, obj)) {
+ DRM_DEBUG_KMS("intel fb init failed\n");
+ goto out_unref_obj;
+ }
+
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ int ret;
+
+ if ((active & (1 << to_intel_crtc(crtc)->pipe)) == 0)
+ continue;
+
+ ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
+ if (ret)
+ goto out_unref_obj;
+
+ crtc->fb = &ifbdev->ifb.base;
+ }
+
+ dev_priv->fbdev = ifbdev;
+
+ DRM_DEBUG_KMS("using BIOS fb for initial console\n");
+ return;
+
+out_unref_obj:
+ drm_gem_object_unreference(&obj->base);
+out_free_ifbdev:
+ kfree(ifbdev);
+}
+
+int intel_fbdev_init(struct drm_device *dev)
+{
+ drm_i915_private_t *dev_priv = dev->dev_private;
+ struct intel_fbdev *ifbdev;
+ int ret;
+
+ if ((ifbdev = dev_priv->fbdev) == NULL) {
+ ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
+ if (ifbdev == NULL)
+ return -ENOMEM;
+
+ ifbdev->helper.funcs = &intel_fb_helper_funcs;
+ ifbdev->preferred_bpp = 32;
+
+ dev_priv->fbdev = ifbdev;
+ }
ret = drm_fb_helper_init(dev, &ifbdev->helper,
- dev_priv->num_pipe,
- INTELFB_CONN_LIMIT);
+ dev_priv->num_pipe,
+ INTELFB_CONN_LIMIT);
if (ret) {
+ dev_priv->fbdev = NULL;
kfree(ifbdev);
return ret;
}
@@ -270,9 +529,10 @@ int intel_fbdev_init(struct drm_device *dev)
void intel_fbdev_initial_config(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
+ struct intel_fbdev *ifbdev = dev_priv->fbdev;
/* Due to peculiar init order wrt to hpd handling this is separate. */
- drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32);
+ drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp);
}
void intel_fbdev_fini(struct drm_device *dev)
@@ -300,7 +560,8 @@ MODULE_LICENSE("GPL and additional rights");
void intel_fb_output_poll_changed(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
- drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
+ if (dev_priv->fbdev)
+ drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
}
void intel_fb_restore_mode(struct drm_device *dev)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon
2013-02-06 11:10 ` [PATCH 5/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon Chris Wilson
@ 2013-03-08 12:05 ` Imre Deak
0 siblings, 0 replies; 20+ messages in thread
From: Imre Deak @ 2013-03-08 12:05 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, 2013-02-06 at 11:10 +0000, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 8 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 14 +-
> drivers/gpu/drm/i915/intel_drv.h | 4 +
> drivers/gpu/drm/i915/intel_fb.c | 305 +++++++++++++++++++++++++++++++---
> 5 files changed, 306 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 4fa6beb..f2b7db7 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1273,6 +1273,7 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
> static int i915_load_modeset_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> + bool was_vga_enabled;
> int ret;
>
> ret = intel_parse_bios(dev);
> @@ -1309,7 +1310,11 @@ static int i915_load_modeset_init(struct drm_device *dev)
>
> /* Important: The output setup functions called by modeset_init need
> * working irqs for e.g. gmbus and dp aux transfers. */
> - intel_modeset_init(dev);
> + intel_modeset_init(dev, &was_vga_enabled);
> +
> + /* Wrap existing BIOS mode configuration prior to GEM takeover */
> + if (!was_vga_enabled)
> + intel_fbdev_init_bios(dev);
>
> ret = i915_gem_init(dev);
> if (ret)
> @@ -1323,6 +1328,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
> /* FIXME: do pre/post-mode set stuff in core KMS code */
> dev->vblank_disable_allowed = 1;
>
> + /* Install a default KMS/GEM fbcon if we failed to wrap the BIOS fb */
> ret = intel_fbdev_init(dev);
> if (ret)
> goto cleanup_gem;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index fd8a495..6f4afbf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1814,7 +1814,7 @@ static inline void intel_unregister_dsm_handler(void) { return; }
>
> /* modesetting */
> extern void intel_modeset_init_hw(struct drm_device *dev);
> -extern void intel_modeset_init(struct drm_device *dev);
> +extern void intel_modeset_init(struct drm_device *dev, bool *was_vga_enabled);
> extern void intel_modeset_gem_init(struct drm_device *dev);
> extern void intel_modeset_cleanup(struct drm_device *dev);
> extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8f9cdd7..7c4c7d5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8698,12 +8698,17 @@ static void intel_init_quirks(struct drm_device *dev)
> }
>
> /* Disable the VGA plane that we never use */
> -static void i915_disable_vga(struct drm_device *dev)
> +static bool i915_disable_vga(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> + bool was_enabled;
> u8 sr1;
> u32 vga_reg = i915_vgacntrl_reg(dev);
>
> + was_enabled = !(I915_READ(vga_reg) & VGA_DISP_DISABLE);
> + DRM_DEBUG_KMS("VGA output is currently %s\n",
> + was_enabled ? "enabled" : "disabled");
> +
> vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
> outb(SR01, VGA_SR_INDEX);
> sr1 = inb(VGA_SR_DATA);
> @@ -8713,6 +8718,8 @@ static void i915_disable_vga(struct drm_device *dev)
>
> I915_WRITE(vga_reg, VGA_DISP_DISABLE);
> POSTING_READ(vga_reg);
> +
> + return was_enabled;
> }
>
> void intel_modeset_init_hw(struct drm_device *dev)
> @@ -8728,7 +8735,8 @@ void intel_modeset_init_hw(struct drm_device *dev)
> mutex_unlock(&dev->struct_mutex);
> }
>
> -void intel_modeset_init(struct drm_device *dev)
> +void intel_modeset_init(struct drm_device *dev,
> + bool *was_vga_enabled)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> int i, ret;
> @@ -8775,7 +8783,7 @@ void intel_modeset_init(struct drm_device *dev)
> intel_pch_pll_init(dev);
>
> /* Just disable it once at startup */
> - i915_disable_vga(dev);
> + *was_vga_enabled = i915_disable_vga(dev);
> intel_setup_outputs(dev);
>
> /* Just in case the BIOS is doing something questionable. */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 07d95a1..985e9dd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -146,6 +146,8 @@ struct intel_fbdev {
> struct intel_framebuffer ifb;
> struct list_head fbdev_list;
> struct drm_display_mode *our_mode;
> + bool stolen;
> + int preferred_bpp;
> };
>
> struct intel_encoder {
> @@ -212,6 +214,7 @@ struct intel_crtc {
> enum plane plane;
> enum transcoder cpu_transcoder;
> u8 lut_r[256], lut_g[256], lut_b[256];
> + bool mode_valid;
> /*
> * Whether the crtc and the connected output pipeline is active. Implies
> * that crtc->enabled is set, i.e. the current mode configuration has
> @@ -612,6 +615,7 @@ extern int intel_framebuffer_init(struct drm_device *dev,
> struct intel_framebuffer *ifb,
> struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_i915_gem_object *obj);
> +extern void intel_fbdev_init_bios(struct drm_device *dev);
> extern int intel_fbdev_init(struct drm_device *dev);
> extern void intel_fbdev_initial_config(struct drm_device *dev);
> extern void intel_fbdev_fini(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index de0ac4c..ca6c8e6 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -131,7 +131,6 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
> struct drm_device *dev = ifbdev->helper.dev;
> struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> struct drm_i915_gem_object *obj;
> - struct fb_info *info;
> int size, ret;
>
> /* we don't do packed 24bpp */
> @@ -176,14 +175,7 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
> mode_cmd.width, mode_cmd.height,
> obj->gtt_offset, obj);
>
> - info = intelfb_create_info(ifbdev);
> - if (info == NULL) {
> - ret = -ENOMEM;
> - goto out_unpin;
> - }
> -
> mutex_unlock(&dev->struct_mutex);
> - vga_switcheroo_client_fb_set(dev->pdev, info);
> return 0;
>
> out_unpin:
> @@ -200,17 +192,92 @@ static int intel_fb_find_or_create_single(struct drm_fb_helper *helper,
> {
> struct intel_fbdev *ifbdev = (struct intel_fbdev *)helper;
> int new_fb = 0;
> - int ret;
>
> if (!helper->fb) {
> - ret = intelfb_create(ifbdev, sizes);
> - if (ret)
> - return ret;
> + struct fb_info *info;
> +
> + if (!ifbdev->stolen) {
> + int ret = intelfb_create(ifbdev, sizes);
> + if (ret)
> + return ret;
> + }
> +
> + info = intelfb_create_info(ifbdev);
> + if (info == NULL) {
> + DRM_DEBUG_KMS("fb creation failed\n");
> + return -ENOMEM;
> + }
> + vga_switcheroo_client_fb_set(helper->dev->pdev, info);
> +
> new_fb = 1;
> }
> +
> return new_fb;
> }
>
> +static struct drm_fb_helper_crtc *
> +intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
> +{
> + int i;
> +
> + for (i = 0; i < fb_helper->crtc_count; i++)
> + if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
> + return &fb_helper->crtc_info[i];
> +
> + return NULL;
> +}
> +
> +static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> + struct drm_fb_helper_crtc **crtcs,
> + struct drm_display_mode **modes,
> + bool *enabled, int width, int height)
> +{
> + int i;
> +
> + for (i = 0; i < fb_helper->connector_count; i++) {
> + struct drm_connector *connector;
> + struct drm_encoder *encoder;
> +
> + connector = fb_helper->connector_info[i]->connector;
> + if (!enabled[i]) {
> + DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
> + connector->base.id);
> + continue;
> + }
> +
> + encoder = connector->encoder;
> + if (!encoder || !encoder->crtc) {
> + DRM_DEBUG_KMS("connector %d has no encoder or crtc, skipping\n",
> + connector->base.id);
> + continue;
> + }
> +
> + if (WARN_ON(!encoder->crtc->enabled)) {
> + DRM_DEBUG_KMS("connector %s on crtc %d has inconsistent state, aborting\n",
> + drm_get_connector_name(connector),
> + encoder->crtc->base.id);
> + return false;
> + }
> +
> + if (!to_intel_crtc(encoder->crtc)->mode_valid) {
> + DRM_DEBUG_KMS("connector %s on crtc %d has an invalid mode, aborting\n",
> + drm_get_connector_name(connector),
> + encoder->crtc->base.id);
> + return false;
> + }
> +
> + modes[i] = &encoder->crtc->mode;
I might be missing something, but I can't see how crtc->mode will be
valid at this point. We'll get here through
intel_fb_initial_config()->drm_setup_crtcs() but as I understand
crtc->mode will only be set afterwards through
intel_fb_initial_config()->drm_fb_helper_single_fb_probe().
At least testing on my ILK I get here with mode_valid=false.
--Imre
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/8] drm/i915: Retrieve the current mode upon KMS takeover
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
` (3 preceding siblings ...)
2013-02-06 11:10 ` [PATCH 5/8] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon Chris Wilson
@ 2013-02-06 11:10 ` Chris Wilson
2013-02-07 12:58 ` Paulo Zanoni
` (2 more replies)
2013-02-06 11:10 ` [PATCH 7/8] drm/i915: Only preserve the BIOS modes if they are the preferred ones Chris Wilson
` (4 subsequent siblings)
9 siblings, 3 replies; 20+ messages in thread
From: Chris Wilson @ 2013-02-06 11:10 UTC (permalink / raw)
To: intel-gfx
Read the current hardware state to retrieve the active mode and populate
our CRTC config if that mode matches our presumptions.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/intel_crt.c | 27 +++++++-
drivers/gpu/drm/i915/intel_display.c | 119 ++++++++++++++++++++++++++--------
drivers/gpu/drm/i915/intel_dp.c | 22 +++++++
drivers/gpu/drm/i915/intel_drv.h | 7 +-
drivers/gpu/drm/i915/intel_dvo.c | 36 ++++++----
drivers/gpu/drm/i915/intel_hdmi.c | 22 +++++++
drivers/gpu/drm/i915/intel_lvds.c | 27 +++++++-
drivers/gpu/drm/i915/intel_sdvo.c | 23 +++++++
9 files changed, 242 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6f4afbf..aff1436 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -280,6 +280,8 @@ struct drm_i915_display_funcs {
void (*update_linetime_wm)(struct drm_device *dev, int pipe,
struct drm_display_mode *mode);
void (*modeset_global_resources)(struct drm_device *dev);
+ bool (*crtc_get_mode)(struct drm_crtc *crtc,
+ struct drm_display_mode *mode);
int (*crtc_mode_set)(struct drm_crtc *crtc,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode,
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 68e79f3..aa3001a 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -81,6 +81,27 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
return true;
}
+static unsigned intel_crt_get_mode_flags(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ struct intel_crt *crt = intel_encoder_to_crt(encoder);
+ u32 tmp, flags = 0;
+
+ tmp = I915_READ(crt->adpa_reg);
+
+ if (tmp & ADPA_HSYNC_ACTIVE_HIGH)
+ flags |= DRM_MODE_FLAG_PHSYNC;
+ else
+ flags |= DRM_MODE_FLAG_NHSYNC;
+
+ if (tmp & ADPA_VSYNC_ACTIVE_HIGH)
+ flags |= DRM_MODE_FLAG_PVSYNC;
+ else
+ flags |= DRM_MODE_FLAG_NVSYNC;
+
+ return flags;
+}
+
static void intel_disable_crt(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -777,10 +798,12 @@ void intel_crt_init(struct drm_device *dev)
crt->base.disable = intel_disable_crt;
crt->base.enable = intel_enable_crt;
- if (HAS_DDI(dev))
+ if (HAS_DDI(dev)) {
crt->base.get_hw_state = intel_ddi_get_hw_state;
- else
+ } else {
crt->base.get_hw_state = intel_crt_get_hw_state;
+ crt->base.get_mode_flags = intel_crt_get_mode_flags;
+ }
intel_connector->get_hw_state = intel_connector_get_hw_state;
drm_encoder_helper_add(&crt->base.base, &crt_encoder_funcs);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7c4c7d5..43e226d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6600,11 +6600,12 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
}
/* Returns the clock of the currently programmed mode of the given pipe. */
-static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
+static int i9xx_crtc_clock_get(struct drm_crtc *crtc)
{
+ struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int pipe = intel_crtc->pipe;
+ enum pipe pipe = intel_crtc->pipe;
u32 dpll = I915_READ(DPLL(pipe));
u32 fp;
intel_clock_t clock;
@@ -6687,35 +6688,84 @@ static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
}
/** Returns the currently programmed mode of the given pipe. */
-struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
- struct drm_crtc *crtc)
+static bool i9xx_crtc_get_mode(struct drm_crtc *crtc,
+ struct drm_display_mode *mode)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_i915_private *dev_priv = crtc->dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
- struct drm_display_mode *mode;
- int htot = I915_READ(HTOTAL(cpu_transcoder));
- int hsync = I915_READ(HSYNC(cpu_transcoder));
- int vtot = I915_READ(VTOTAL(cpu_transcoder));
- int vsync = I915_READ(VSYNC(cpu_transcoder));
+ u32 tmp;
- mode = kzalloc(sizeof(*mode), GFP_KERNEL);
- if (!mode)
- return NULL;
+ memset(mode, 0, sizeof(*mode));
+
+ tmp = I915_READ(HTOTAL(cpu_transcoder));
+ mode->hdisplay = (tmp & 0xffff) + 1;
+ mode->htotal = ((tmp & 0xffff0000) >> 16) + 1;
+
+ tmp = I915_READ(HSYNC(cpu_transcoder));
+ mode->hsync_start = (tmp & 0xffff) + 1;
+ mode->hsync_end = ((tmp & 0xffff0000) >> 16) + 1;
+
+ tmp = I915_READ(VTOTAL(cpu_transcoder));
+ mode->vdisplay = (tmp & 0xffff) + 1;
+ mode->vtotal = ((tmp & 0xffff0000) >> 16) + 1;
+
+ tmp = I915_READ(VSYNC(cpu_transcoder));
+ mode->vsync_start = (tmp & 0xffff) + 1;
+ mode->vsync_end = ((tmp & 0xffff0000) >> 16) + 1;
+
+ mode->clock = i9xx_crtc_clock_get(crtc);
+
+ drm_mode_set_name(mode);
+
+ return true;
+}
+
+static bool ironlake_crtc_get_mode(struct drm_crtc *crtc,
+ struct drm_display_mode *mode)
+{
+ struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
+ u32 tmp;
+
+ memset(mode, 0, sizeof(*mode));
+
+ tmp = I915_READ(HTOTAL(cpu_transcoder));
+ mode->hdisplay = (tmp & 0xffff) + 1;
+ mode->htotal = ((tmp & 0xffff0000) >> 16) + 1;
+
+ tmp = I915_READ(HSYNC(cpu_transcoder));
+ mode->hsync_start = (tmp & 0xffff) + 1;
+ mode->hsync_end = ((tmp & 0xffff0000) >> 16) + 1;
+
+ tmp = I915_READ(VTOTAL(cpu_transcoder));
+ mode->vdisplay = (tmp & 0xffff) + 1;
+ mode->vtotal = ((tmp & 0xffff0000) >> 16) + 1;
- mode->clock = intel_crtc_clock_get(dev, crtc);
- mode->hdisplay = (htot & 0xffff) + 1;
- mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
- mode->hsync_start = (hsync & 0xffff) + 1;
- mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
- mode->vdisplay = (vtot & 0xffff) + 1;
- mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
- mode->vsync_start = (vsync & 0xffff) + 1;
- mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
+ tmp = I915_READ(VSYNC(cpu_transcoder));
+ mode->vsync_start = (tmp & 0xffff) + 1;
+ mode->vsync_end = ((tmp & 0xffff0000) >> 16) + 1;
+
+ //mode->clock = i9xx_crtc_clock_get(crtc);
+ //mode->clock = 69300;
drm_mode_set_name(mode);
- return mode;
+ return false; /* XXX mode->clock unset */
+}
+
+static __maybe_unused bool no_crtc_get_mode(struct drm_crtc *crtc,
+ struct drm_display_mode *mode)
+{
+ return false;
+}
+
+bool intel_crtc_get_mode(struct drm_crtc *crtc,
+ struct drm_display_mode *mode)
+{
+ struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+ return dev_priv->display.crtc_get_mode(crtc, mode);
}
static void intel_increase_pllclock(struct drm_crtc *crtc)
@@ -8478,20 +8528,25 @@ static void intel_init_display(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ dev_priv->display.crtc_get_mode = no_crtc_get_mode;
+
/* We always want a DPMS function */
if (HAS_DDI(dev)) {
+ dev_priv->display.crtc_get_mode = ironlake_crtc_get_mode;
dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
dev_priv->display.crtc_enable = haswell_crtc_enable;
dev_priv->display.crtc_disable = haswell_crtc_disable;
dev_priv->display.off = haswell_crtc_off;
dev_priv->display.update_plane = ironlake_update_plane;
} else if (HAS_PCH_SPLIT(dev)) {
+ dev_priv->display.crtc_get_mode = ironlake_crtc_get_mode;
dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
dev_priv->display.crtc_enable = ironlake_crtc_enable;
dev_priv->display.crtc_disable = ironlake_crtc_disable;
dev_priv->display.off = ironlake_crtc_off;
dev_priv->display.update_plane = ironlake_update_plane;
} else {
+ dev_priv->display.crtc_get_mode = i9xx_crtc_get_mode;
dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
dev_priv->display.crtc_enable = i9xx_crtc_enable;
dev_priv->display.crtc_disable = i9xx_crtc_disable;
@@ -9031,6 +9086,14 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
crtc->base.base.id,
crtc->active ? "enabled" : "disabled");
+
+
+ if (crtc->base.enabled)
+ crtc->mode_valid = intel_crtc_get_mode(&crtc->base, &crtc->base.mode);
+ if (crtc->mode_valid) {
+ DRM_DEBUG_KMS("found active mode: ");
+ drm_mode_debug_printmodeline(&crtc->base.mode);
+ }
}
if (HAS_DDI(dev))
@@ -9038,12 +9101,14 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
list_for_each_entry(encoder, &dev->mode_config.encoder_list,
base.head) {
- pipe = 0;
+ pipe = -1;
if (encoder->get_hw_state(encoder, &pipe)) {
- encoder->base.crtc =
- dev_priv->pipe_to_crtc_mapping[pipe];
- } else {
+ crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+ if (crtc->mode_valid && encoder->get_mode_flags)
+ crtc->base.mode.flags |= encoder->get_mode_flags(encoder);
+ encoder->base.crtc = &crtc->base;
+ } else
encoder->base.crtc = NULL;
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1b76b04..c82aed3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1409,6 +1409,27 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
return true;
}
+static unsigned intel_dp_get_mode_flags(struct intel_encoder *encoder)
+{
+ struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ u32 tmp, flags = 0;
+
+ tmp = I915_READ(intel_dp->output_reg);
+
+ if (tmp & DP_SYNC_HS_HIGH)
+ flags |= DRM_MODE_FLAG_PHSYNC;
+ else
+ flags |= DRM_MODE_FLAG_NHSYNC;
+
+ if (tmp & DP_SYNC_VS_HIGH)
+ flags |= DRM_MODE_FLAG_PVSYNC;
+ else
+ flags |= DRM_MODE_FLAG_NVSYNC;
+
+ return flags;
+}
+
static void intel_disable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -2947,6 +2968,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
intel_encoder->disable = intel_disable_dp;
intel_encoder->post_disable = intel_post_disable_dp;
intel_encoder->get_hw_state = intel_dp_get_hw_state;
+ intel_encoder->get_mode_flags = intel_dp_get_mode_flags;
intel_dig_port->port = port;
intel_dig_port->dp.output_reg = output_reg;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 985e9dd..a6c0b25 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -176,6 +176,9 @@ struct intel_encoder {
* the encoder is active. If the encoder is enabled it also set the pipe
* it is connected to in the pipe parameter. */
bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
+ /* Reconstructs the equivalent mode flags for the current hardware
+ * state. */
+ unsigned (*get_mode_flags)(struct intel_encoder *);
int crtc_mask;
};
@@ -577,8 +580,8 @@ extern void intel_connector_attach_encoder(struct intel_connector *connector,
struct intel_encoder *encoder);
extern struct drm_encoder *intel_best_encoder(struct drm_connector *connector);
-extern struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
- struct drm_crtc *crtc);
+extern bool intel_crtc_get_mode(struct drm_crtc *crtc,
+ struct drm_display_mode *mode);
int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
struct drm_file *file_priv);
extern enum transcoder
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 15da995..46b549e 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -129,6 +129,22 @@ static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
return true;
}
+static unsigned
+intel_dvo_get_mode_flags(struct intel_encoder *encoder)
+{
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ struct intel_dvo *intel_dvo = enc_to_intel_dvo(&encoder->base);
+ u32 tmp, flags = 0;
+
+ tmp = I915_READ(intel_dvo->dev.dvo_reg);
+ if (tmp & DVO_HSYNC_ACTIVE_HIGH)
+ flags |= DRM_MODE_FLAG_PHSYNC;
+ if (tmp & DVO_VSYNC_ACTIVE_HIGH)
+ flags |= DRM_MODE_FLAG_PVSYNC;
+
+ return flags;
+}
+
static void intel_disable_dvo(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -390,29 +406,26 @@ intel_dvo_get_current_mode(struct drm_connector *connector)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
uint32_t dvo_val = I915_READ(intel_dvo->dev.dvo_reg);
- struct drm_display_mode *mode = NULL;
+ struct drm_display_mode *fixed_mode = NULL;
/* If the DVO port is active, that'll be the LVDS, so we can pull out
* its timings to get how the BIOS set up the panel.
*/
if (dvo_val & DVO_ENABLE) {
+ struct drm_display_mode mode;
struct drm_crtc *crtc;
int pipe = (dvo_val & DVO_PIPE_B_SELECT) ? 1 : 0;
crtc = intel_get_crtc_for_pipe(dev, pipe);
- if (crtc) {
- mode = intel_crtc_mode_get(dev, crtc);
- if (mode) {
- mode->type |= DRM_MODE_TYPE_PREFERRED;
- if (dvo_val & DVO_HSYNC_ACTIVE_HIGH)
- mode->flags |= DRM_MODE_FLAG_PHSYNC;
- if (dvo_val & DVO_VSYNC_ACTIVE_HIGH)
- mode->flags |= DRM_MODE_FLAG_PVSYNC;
- }
+ if (intel_crtc_get_mode(crtc, &mode))
+ fixed_mode = drm_mode_duplicate(dev, &mode);
+ if (fixed_mode) {
+ fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
+ fixed_mode->flags = intel_dvo_get_mode_flags(&intel_dvo->base);
}
}
- return mode;
+ return fixed_mode;
}
void intel_dvo_init(struct drm_device *dev)
@@ -441,6 +454,7 @@ void intel_dvo_init(struct drm_device *dev)
intel_encoder->disable = intel_disable_dvo;
intel_encoder->enable = intel_enable_dvo;
intel_encoder->get_hw_state = intel_dvo_get_hw_state;
+ intel_encoder->get_mode_flags = intel_dvo_get_mode_flags;
intel_connector->get_hw_state = intel_dvo_connector_get_hw_state;
/* Now, try to find a controller */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 5b4efd6..94e3d5d 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -656,6 +656,27 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
return true;
}
+static unsigned intel_hdmi_get_mode_flags(struct intel_encoder *encoder)
+{
+ struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ u32 tmp, flags = 0;
+
+ tmp = I915_READ(intel_hdmi->sdvox_reg);
+
+ if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
+ flags |= DRM_MODE_FLAG_PHSYNC;
+ else
+ flags |= DRM_MODE_FLAG_NHSYNC;
+
+ if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
+ flags |= DRM_MODE_FLAG_PVSYNC;
+ else
+ flags |= DRM_MODE_FLAG_NVSYNC;
+
+ return flags;
+}
+
static void intel_enable_hdmi(struct intel_encoder *encoder)
{
struct drm_device *dev = encoder->base.dev;
@@ -1102,6 +1123,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
intel_encoder->enable = intel_enable_hdmi;
intel_encoder->disable = intel_disable_hdmi;
intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
+ intel_encoder->get_mode_flags = intel_hdmi_get_mode_flags;
intel_encoder->type = INTEL_OUTPUT_HDMI;
intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 7e4ec63..3da1b2a 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -89,6 +89,26 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
return true;
}
+static unsigned intel_lvds_get_mode_flags(struct intel_encoder *encoder)
+{
+ struct drm_device *dev = encoder->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 lvds_reg, tmp, flags = 0;
+
+ if (HAS_PCH_SPLIT(dev))
+ lvds_reg = PCH_LVDS;
+ else
+ lvds_reg = LVDS;
+
+ tmp = I915_READ(lvds_reg);
+ if (tmp & LVDS_HSYNC_POLARITY)
+ flags |= DRM_MODE_FLAG_NHSYNC;
+ if (tmp & LVDS_VSYNC_POLARITY)
+ flags |= DRM_MODE_FLAG_NVSYNC;
+
+ return flags;
+}
+
/* The LVDS pin pair needs to be on before the DPLLs are enabled.
* This is an exception to the general rule that mode_set doesn't turn
* things on.
@@ -1112,6 +1132,7 @@ bool intel_lvds_init(struct drm_device *dev)
intel_encoder->pre_pll_enable = intel_pre_pll_enable_lvds;
intel_encoder->disable = intel_disable_lvds;
intel_encoder->get_hw_state = intel_lvds_get_hw_state;
+ intel_encoder->get_mode_flags = intel_lvds_get_mode_flags;
intel_connector->get_hw_state = intel_connector_get_hw_state;
intel_connector_attach_encoder(intel_connector, intel_encoder);
@@ -1223,11 +1244,15 @@ bool intel_lvds_init(struct drm_device *dev)
crtc = intel_get_crtc_for_pipe(dev, pipe);
if (crtc && (lvds & LVDS_PORT_EN)) {
- fixed_mode = intel_crtc_mode_get(dev, crtc);
+ struct drm_display_mode mode;
+
+ if (intel_crtc_get_mode(crtc, &mode))
+ fixed_mode = drm_mode_duplicate(dev, &mode);
if (fixed_mode) {
DRM_DEBUG_KMS("using current (BIOS) mode: ");
drm_mode_debug_printmodeline(fixed_mode);
fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
+ fixed_mode->flags = intel_lvds_get_mode_flags(intel_encoder);
goto out;
}
}
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f01063a..918c269 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1250,6 +1250,27 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
return true;
}
+static unsigned intel_sdvo_get_mode_flags(struct intel_encoder *encoder)
+{
+ struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
+ struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+ u32 tmp, flags = 0;
+
+ tmp = I915_READ(intel_sdvo->sdvo_reg);
+
+ if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
+ flags |= DRM_MODE_FLAG_PHSYNC;
+ else
+ flags |= DRM_MODE_FLAG_NHSYNC;
+
+ if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
+ flags |= DRM_MODE_FLAG_PVSYNC;
+ else
+ flags |= DRM_MODE_FLAG_NVSYNC;
+
+ return flags;
+}
+
static void intel_disable_sdvo(struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -2785,6 +2806,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
intel_encoder->disable = intel_disable_sdvo;
intel_encoder->enable = intel_enable_sdvo;
intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
+ if (INTEL_INFO(dev)->gen >= 4)
+ intel_encoder->get_mode_flags = intel_sdvo_get_mode_flags;
/* In default case sdvo lvds is false */
if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 6/8] drm/i915: Retrieve the current mode upon KMS takeover
2013-02-06 11:10 ` [PATCH 6/8] drm/i915: Retrieve the current mode upon KMS takeover Chris Wilson
@ 2013-02-07 12:58 ` Paulo Zanoni
2013-03-08 15:46 ` Imre Deak
2013-03-08 15:51 ` Imre Deak
2 siblings, 0 replies; 20+ messages in thread
From: Paulo Zanoni @ 2013-02-07 12:58 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
Hi
2013/2/6 Chris Wilson <chris@chris-wilson.co.uk>:
> Read the current hardware state to retrieve the active mode and populate
> our CRTC config if that mode matches our presumptions.
On Haswell you need to read TRANS_DDI_FUNC_CTL(cpu_transcoder), check
for bits TRANS_DDI_PVSYNC and TRANS_DDI_PHSYNC.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/intel_crt.c | 27 +++++++-
> drivers/gpu/drm/i915/intel_display.c | 119 ++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/intel_dp.c | 22 +++++++
> drivers/gpu/drm/i915/intel_drv.h | 7 +-
> drivers/gpu/drm/i915/intel_dvo.c | 36 ++++++----
> drivers/gpu/drm/i915/intel_hdmi.c | 22 +++++++
> drivers/gpu/drm/i915/intel_lvds.c | 27 +++++++-
> drivers/gpu/drm/i915/intel_sdvo.c | 23 +++++++
> 9 files changed, 242 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6f4afbf..aff1436 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -280,6 +280,8 @@ struct drm_i915_display_funcs {
> void (*update_linetime_wm)(struct drm_device *dev, int pipe,
> struct drm_display_mode *mode);
> void (*modeset_global_resources)(struct drm_device *dev);
> + bool (*crtc_get_mode)(struct drm_crtc *crtc,
> + struct drm_display_mode *mode);
> int (*crtc_mode_set)(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> struct drm_display_mode *adjusted_mode,
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 68e79f3..aa3001a 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -81,6 +81,27 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_crt_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_crt *crt = intel_encoder_to_crt(encoder);
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(crt->adpa_reg);
> +
> + if (tmp & ADPA_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & ADPA_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_crt(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -777,10 +798,12 @@ void intel_crt_init(struct drm_device *dev)
>
> crt->base.disable = intel_disable_crt;
> crt->base.enable = intel_enable_crt;
> - if (HAS_DDI(dev))
> + if (HAS_DDI(dev)) {
> crt->base.get_hw_state = intel_ddi_get_hw_state;
> - else
> + } else {
> crt->base.get_hw_state = intel_crt_get_hw_state;
> + crt->base.get_mode_flags = intel_crt_get_mode_flags;
> + }
> intel_connector->get_hw_state = intel_connector_get_hw_state;
>
> drm_encoder_helper_add(&crt->base.base, &crt_encoder_funcs);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7c4c7d5..43e226d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6600,11 +6600,12 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> }
>
> /* Returns the clock of the currently programmed mode of the given pipe. */
> -static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
> +static int i9xx_crtc_clock_get(struct drm_crtc *crtc)
> {
> + struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pipe = intel_crtc->pipe;
> + enum pipe pipe = intel_crtc->pipe;
> u32 dpll = I915_READ(DPLL(pipe));
> u32 fp;
> intel_clock_t clock;
> @@ -6687,35 +6688,84 @@ static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
> }
>
> /** Returns the currently programmed mode of the given pipe. */
> -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> - struct drm_crtc *crtc)
> +static bool i9xx_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> - struct drm_display_mode *mode;
> - int htot = I915_READ(HTOTAL(cpu_transcoder));
> - int hsync = I915_READ(HSYNC(cpu_transcoder));
> - int vtot = I915_READ(VTOTAL(cpu_transcoder));
> - int vsync = I915_READ(VSYNC(cpu_transcoder));
> + u32 tmp;
>
> - mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> - if (!mode)
> - return NULL;
> + memset(mode, 0, sizeof(*mode));
> +
> + tmp = I915_READ(HTOTAL(cpu_transcoder));
> + mode->hdisplay = (tmp & 0xffff) + 1;
> + mode->htotal = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(HSYNC(cpu_transcoder));
> + mode->hsync_start = (tmp & 0xffff) + 1;
> + mode->hsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(VTOTAL(cpu_transcoder));
> + mode->vdisplay = (tmp & 0xffff) + 1;
> + mode->vtotal = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(VSYNC(cpu_transcoder));
> + mode->vsync_start = (tmp & 0xffff) + 1;
> + mode->vsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + mode->clock = i9xx_crtc_clock_get(crtc);
> +
> + drm_mode_set_name(mode);
> +
> + return true;
> +}
> +
> +static bool ironlake_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> + u32 tmp;
> +
> + memset(mode, 0, sizeof(*mode));
> +
> + tmp = I915_READ(HTOTAL(cpu_transcoder));
> + mode->hdisplay = (tmp & 0xffff) + 1;
> + mode->htotal = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(HSYNC(cpu_transcoder));
> + mode->hsync_start = (tmp & 0xffff) + 1;
> + mode->hsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(VTOTAL(cpu_transcoder));
> + mode->vdisplay = (tmp & 0xffff) + 1;
> + mode->vtotal = ((tmp & 0xffff0000) >> 16) + 1;
>
> - mode->clock = intel_crtc_clock_get(dev, crtc);
> - mode->hdisplay = (htot & 0xffff) + 1;
> - mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
> - mode->hsync_start = (hsync & 0xffff) + 1;
> - mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
> - mode->vdisplay = (vtot & 0xffff) + 1;
> - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
> - mode->vsync_start = (vsync & 0xffff) + 1;
> - mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
> + tmp = I915_READ(VSYNC(cpu_transcoder));
> + mode->vsync_start = (tmp & 0xffff) + 1;
> + mode->vsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + //mode->clock = i9xx_crtc_clock_get(crtc);
> + //mode->clock = 69300;
>
> drm_mode_set_name(mode);
>
> - return mode;
> + return false; /* XXX mode->clock unset */
> +}
> +
> +static __maybe_unused bool no_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + return false;
> +}
> +
> +bool intel_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + return dev_priv->display.crtc_get_mode(crtc, mode);
> }
>
> static void intel_increase_pllclock(struct drm_crtc *crtc)
> @@ -8478,20 +8528,25 @@ static void intel_init_display(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> + dev_priv->display.crtc_get_mode = no_crtc_get_mode;
> +
> /* We always want a DPMS function */
> if (HAS_DDI(dev)) {
> + dev_priv->display.crtc_get_mode = ironlake_crtc_get_mode;
> dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
> dev_priv->display.crtc_enable = haswell_crtc_enable;
> dev_priv->display.crtc_disable = haswell_crtc_disable;
> dev_priv->display.off = haswell_crtc_off;
> dev_priv->display.update_plane = ironlake_update_plane;
> } else if (HAS_PCH_SPLIT(dev)) {
> + dev_priv->display.crtc_get_mode = ironlake_crtc_get_mode;
> dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
> dev_priv->display.crtc_enable = ironlake_crtc_enable;
> dev_priv->display.crtc_disable = ironlake_crtc_disable;
> dev_priv->display.off = ironlake_crtc_off;
> dev_priv->display.update_plane = ironlake_update_plane;
> } else {
> + dev_priv->display.crtc_get_mode = i9xx_crtc_get_mode;
> dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
> dev_priv->display.crtc_enable = i9xx_crtc_enable;
> dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -9031,6 +9086,14 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> crtc->base.base.id,
> crtc->active ? "enabled" : "disabled");
> +
> +
> + if (crtc->base.enabled)
> + crtc->mode_valid = intel_crtc_get_mode(&crtc->base, &crtc->base.mode);
> + if (crtc->mode_valid) {
> + DRM_DEBUG_KMS("found active mode: ");
> + drm_mode_debug_printmodeline(&crtc->base.mode);
> + }
> }
>
> if (HAS_DDI(dev))
> @@ -9038,12 +9101,14 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>
> list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> base.head) {
> - pipe = 0;
> + pipe = -1;
>
> if (encoder->get_hw_state(encoder, &pipe)) {
> - encoder->base.crtc =
> - dev_priv->pipe_to_crtc_mapping[pipe];
> - } else {
> + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> + if (crtc->mode_valid && encoder->get_mode_flags)
> + crtc->base.mode.flags |= encoder->get_mode_flags(encoder);
> + encoder->base.crtc = &crtc->base;
> + } else
> encoder->base.crtc = NULL;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1b76b04..c82aed3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1409,6 +1409,27 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_dp_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_dp->output_reg);
> +
> + if (tmp & DP_SYNC_HS_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & DP_SYNC_VS_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_dp(struct intel_encoder *encoder)
> {
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> @@ -2947,6 +2968,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> intel_encoder->disable = intel_disable_dp;
> intel_encoder->post_disable = intel_post_disable_dp;
> intel_encoder->get_hw_state = intel_dp_get_hw_state;
> + intel_encoder->get_mode_flags = intel_dp_get_mode_flags;
>
> intel_dig_port->port = port;
> intel_dig_port->dp.output_reg = output_reg;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 985e9dd..a6c0b25 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -176,6 +176,9 @@ struct intel_encoder {
> * the encoder is active. If the encoder is enabled it also set the pipe
> * it is connected to in the pipe parameter. */
> bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
> + /* Reconstructs the equivalent mode flags for the current hardware
> + * state. */
> + unsigned (*get_mode_flags)(struct intel_encoder *);
> int crtc_mask;
> };
>
> @@ -577,8 +580,8 @@ extern void intel_connector_attach_encoder(struct intel_connector *connector,
> struct intel_encoder *encoder);
> extern struct drm_encoder *intel_best_encoder(struct drm_connector *connector);
>
> -extern struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> - struct drm_crtc *crtc);
> +extern bool intel_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode);
> int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> extern enum transcoder
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 15da995..46b549e 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -129,6 +129,22 @@ static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned
> +intel_dvo_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_dvo *intel_dvo = enc_to_intel_dvo(&encoder->base);
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_dvo->dev.dvo_reg);
> + if (tmp & DVO_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + if (tmp & DVO_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_dvo(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -390,29 +406,26 @@ intel_dvo_get_current_mode(struct drm_connector *connector)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> uint32_t dvo_val = I915_READ(intel_dvo->dev.dvo_reg);
> - struct drm_display_mode *mode = NULL;
> + struct drm_display_mode *fixed_mode = NULL;
>
> /* If the DVO port is active, that'll be the LVDS, so we can pull out
> * its timings to get how the BIOS set up the panel.
> */
> if (dvo_val & DVO_ENABLE) {
> + struct drm_display_mode mode;
> struct drm_crtc *crtc;
> int pipe = (dvo_val & DVO_PIPE_B_SELECT) ? 1 : 0;
>
> crtc = intel_get_crtc_for_pipe(dev, pipe);
> - if (crtc) {
> - mode = intel_crtc_mode_get(dev, crtc);
> - if (mode) {
> - mode->type |= DRM_MODE_TYPE_PREFERRED;
> - if (dvo_val & DVO_HSYNC_ACTIVE_HIGH)
> - mode->flags |= DRM_MODE_FLAG_PHSYNC;
> - if (dvo_val & DVO_VSYNC_ACTIVE_HIGH)
> - mode->flags |= DRM_MODE_FLAG_PVSYNC;
> - }
> + if (intel_crtc_get_mode(crtc, &mode))
> + fixed_mode = drm_mode_duplicate(dev, &mode);
> + if (fixed_mode) {
> + fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> + fixed_mode->flags = intel_dvo_get_mode_flags(&intel_dvo->base);
> }
> }
>
> - return mode;
> + return fixed_mode;
> }
>
> void intel_dvo_init(struct drm_device *dev)
> @@ -441,6 +454,7 @@ void intel_dvo_init(struct drm_device *dev)
> intel_encoder->disable = intel_disable_dvo;
> intel_encoder->enable = intel_enable_dvo;
> intel_encoder->get_hw_state = intel_dvo_get_hw_state;
> + intel_encoder->get_mode_flags = intel_dvo_get_mode_flags;
> intel_connector->get_hw_state = intel_dvo_connector_get_hw_state;
>
> /* Now, try to find a controller */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 5b4efd6..94e3d5d 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -656,6 +656,27 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_hdmi_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_hdmi->sdvox_reg);
> +
> + if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_enable_hdmi(struct intel_encoder *encoder)
> {
> struct drm_device *dev = encoder->base.dev;
> @@ -1102,6 +1123,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
> intel_encoder->enable = intel_enable_hdmi;
> intel_encoder->disable = intel_disable_hdmi;
> intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
> + intel_encoder->get_mode_flags = intel_hdmi_get_mode_flags;
>
> intel_encoder->type = INTEL_OUTPUT_HDMI;
> intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 7e4ec63..3da1b2a 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -89,6 +89,26 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_lvds_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 lvds_reg, tmp, flags = 0;
> +
> + if (HAS_PCH_SPLIT(dev))
> + lvds_reg = PCH_LVDS;
> + else
> + lvds_reg = LVDS;
> +
> + tmp = I915_READ(lvds_reg);
> + if (tmp & LVDS_HSYNC_POLARITY)
> + flags |= DRM_MODE_FLAG_NHSYNC;
> + if (tmp & LVDS_VSYNC_POLARITY)
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> /* The LVDS pin pair needs to be on before the DPLLs are enabled.
> * This is an exception to the general rule that mode_set doesn't turn
> * things on.
> @@ -1112,6 +1132,7 @@ bool intel_lvds_init(struct drm_device *dev)
> intel_encoder->pre_pll_enable = intel_pre_pll_enable_lvds;
> intel_encoder->disable = intel_disable_lvds;
> intel_encoder->get_hw_state = intel_lvds_get_hw_state;
> + intel_encoder->get_mode_flags = intel_lvds_get_mode_flags;
> intel_connector->get_hw_state = intel_connector_get_hw_state;
>
> intel_connector_attach_encoder(intel_connector, intel_encoder);
> @@ -1223,11 +1244,15 @@ bool intel_lvds_init(struct drm_device *dev)
> crtc = intel_get_crtc_for_pipe(dev, pipe);
>
> if (crtc && (lvds & LVDS_PORT_EN)) {
> - fixed_mode = intel_crtc_mode_get(dev, crtc);
> + struct drm_display_mode mode;
> +
> + if (intel_crtc_get_mode(crtc, &mode))
> + fixed_mode = drm_mode_duplicate(dev, &mode);
> if (fixed_mode) {
> DRM_DEBUG_KMS("using current (BIOS) mode: ");
> drm_mode_debug_printmodeline(fixed_mode);
> fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> + fixed_mode->flags = intel_lvds_get_mode_flags(intel_encoder);
> goto out;
> }
> }
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index f01063a..918c269 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1250,6 +1250,27 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_sdvo_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_sdvo->sdvo_reg);
> +
> + if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_sdvo(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -2785,6 +2806,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
> intel_encoder->disable = intel_disable_sdvo;
> intel_encoder->enable = intel_enable_sdvo;
> intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
> + if (INTEL_INFO(dev)->gen >= 4)
> + intel_encoder->get_mode_flags = intel_sdvo_get_mode_flags;
>
> /* In default case sdvo lvds is false */
> if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 6/8] drm/i915: Retrieve the current mode upon KMS takeover
2013-02-06 11:10 ` [PATCH 6/8] drm/i915: Retrieve the current mode upon KMS takeover Chris Wilson
2013-02-07 12:58 ` Paulo Zanoni
@ 2013-03-08 15:46 ` Imre Deak
2013-03-08 15:51 ` Imre Deak
2 siblings, 0 replies; 20+ messages in thread
From: Imre Deak @ 2013-03-08 15:46 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, 2013-02-06 at 11:10 +0000, Chris Wilson wrote:
> Read the current hardware state to retrieve the active mode and populate
> our CRTC config if that mode matches our presumptions.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/intel_crt.c | 27 +++++++-
> drivers/gpu/drm/i915/intel_display.c | 119 ++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/intel_dp.c | 22 +++++++
> drivers/gpu/drm/i915/intel_drv.h | 7 +-
> drivers/gpu/drm/i915/intel_dvo.c | 36 ++++++----
> drivers/gpu/drm/i915/intel_hdmi.c | 22 +++++++
> drivers/gpu/drm/i915/intel_lvds.c | 27 +++++++-
> drivers/gpu/drm/i915/intel_sdvo.c | 23 +++++++
> 9 files changed, 242 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6f4afbf..aff1436 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -280,6 +280,8 @@ struct drm_i915_display_funcs {
> void (*update_linetime_wm)(struct drm_device *dev, int pipe,
> struct drm_display_mode *mode);
> void (*modeset_global_resources)(struct drm_device *dev);
> + bool (*crtc_get_mode)(struct drm_crtc *crtc,
> + struct drm_display_mode *mode);
> int (*crtc_mode_set)(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> struct drm_display_mode *adjusted_mode,
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 68e79f3..aa3001a 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -81,6 +81,27 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_crt_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_crt *crt = intel_encoder_to_crt(encoder);
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(crt->adpa_reg);
> +
> + if (tmp & ADPA_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & ADPA_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_crt(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -777,10 +798,12 @@ void intel_crt_init(struct drm_device *dev)
>
> crt->base.disable = intel_disable_crt;
> crt->base.enable = intel_enable_crt;
> - if (HAS_DDI(dev))
> + if (HAS_DDI(dev)) {
> crt->base.get_hw_state = intel_ddi_get_hw_state;
> - else
> + } else {
> crt->base.get_hw_state = intel_crt_get_hw_state;
> + crt->base.get_mode_flags = intel_crt_get_mode_flags;
> + }
> intel_connector->get_hw_state = intel_connector_get_hw_state;
>
> drm_encoder_helper_add(&crt->base.base, &crt_encoder_funcs);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7c4c7d5..43e226d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6600,11 +6600,12 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> }
>
> /* Returns the clock of the currently programmed mode of the given pipe. */
> -static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
> +static int i9xx_crtc_clock_get(struct drm_crtc *crtc)
> {
> + struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pipe = intel_crtc->pipe;
> + enum pipe pipe = intel_crtc->pipe;
> u32 dpll = I915_READ(DPLL(pipe));
> u32 fp;
> intel_clock_t clock;
> @@ -6687,35 +6688,84 @@ static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
> }
>
> /** Returns the currently programmed mode of the given pipe. */
> -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> - struct drm_crtc *crtc)
> +static bool i9xx_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> - struct drm_display_mode *mode;
> - int htot = I915_READ(HTOTAL(cpu_transcoder));
> - int hsync = I915_READ(HSYNC(cpu_transcoder));
> - int vtot = I915_READ(VTOTAL(cpu_transcoder));
> - int vsync = I915_READ(VSYNC(cpu_transcoder));
> + u32 tmp;
>
> - mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> - if (!mode)
> - return NULL;
> + memset(mode, 0, sizeof(*mode));
> +
> + tmp = I915_READ(HTOTAL(cpu_transcoder));
> + mode->hdisplay = (tmp & 0xffff) + 1;
> + mode->htotal = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(HSYNC(cpu_transcoder));
> + mode->hsync_start = (tmp & 0xffff) + 1;
> + mode->hsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(VTOTAL(cpu_transcoder));
> + mode->vdisplay = (tmp & 0xffff) + 1;
> + mode->vtotal = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(VSYNC(cpu_transcoder));
> + mode->vsync_start = (tmp & 0xffff) + 1;
> + mode->vsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + mode->clock = i9xx_crtc_clock_get(crtc);
> +
> + drm_mode_set_name(mode);
> +
> + return true;
> +}
> +
> +static bool ironlake_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> + u32 tmp;
> +
> + memset(mode, 0, sizeof(*mode));
> +
> + tmp = I915_READ(HTOTAL(cpu_transcoder));
> + mode->hdisplay = (tmp & 0xffff) + 1;
> + mode->htotal = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(HSYNC(cpu_transcoder));
> + mode->hsync_start = (tmp & 0xffff) + 1;
> + mode->hsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(VTOTAL(cpu_transcoder));
> + mode->vdisplay = (tmp & 0xffff) + 1;
> + mode->vtotal = ((tmp & 0xffff0000) >> 16) + 1;
>
> - mode->clock = intel_crtc_clock_get(dev, crtc);
> - mode->hdisplay = (htot & 0xffff) + 1;
> - mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
> - mode->hsync_start = (hsync & 0xffff) + 1;
> - mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
> - mode->vdisplay = (vtot & 0xffff) + 1;
> - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
> - mode->vsync_start = (vsync & 0xffff) + 1;
> - mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
> + tmp = I915_READ(VSYNC(cpu_transcoder));
> + mode->vsync_start = (tmp & 0xffff) + 1;
> + mode->vsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + //mode->clock = i9xx_crtc_clock_get(crtc);
> + //mode->clock = 69300;
>
> drm_mode_set_name(mode);
>
> - return mode;
> + return false; /* XXX mode->clock unset */
> +}
> +
> +static __maybe_unused bool no_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + return false;
> +}
> +
> +bool intel_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + return dev_priv->display.crtc_get_mode(crtc, mode);
> }
>
> static void intel_increase_pllclock(struct drm_crtc *crtc)
> @@ -8478,20 +8528,25 @@ static void intel_init_display(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> + dev_priv->display.crtc_get_mode = no_crtc_get_mode;
> +
> /* We always want a DPMS function */
> if (HAS_DDI(dev)) {
> + dev_priv->display.crtc_get_mode = ironlake_crtc_get_mode;
> dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
> dev_priv->display.crtc_enable = haswell_crtc_enable;
> dev_priv->display.crtc_disable = haswell_crtc_disable;
> dev_priv->display.off = haswell_crtc_off;
> dev_priv->display.update_plane = ironlake_update_plane;
> } else if (HAS_PCH_SPLIT(dev)) {
> + dev_priv->display.crtc_get_mode = ironlake_crtc_get_mode;
> dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
> dev_priv->display.crtc_enable = ironlake_crtc_enable;
> dev_priv->display.crtc_disable = ironlake_crtc_disable;
> dev_priv->display.off = ironlake_crtc_off;
> dev_priv->display.update_plane = ironlake_update_plane;
> } else {
> + dev_priv->display.crtc_get_mode = i9xx_crtc_get_mode;
> dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
> dev_priv->display.crtc_enable = i9xx_crtc_enable;
> dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -9031,6 +9086,14 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> crtc->base.base.id,
> crtc->active ? "enabled" : "disabled");
> +
> +
> + if (crtc->base.enabled)
> + crtc->mode_valid = intel_crtc_get_mode(&crtc->base, &crtc->base.mode);
Ok, this explains the invalid mode I got and so you can ignore my
comment about it in the previous patch. Well, also for ILK there are
still the missing clock query in ironlake_crtc_get_mode(), so I couldn't
test this.
> + if (crtc->mode_valid) {
> + DRM_DEBUG_KMS("found active mode: ");
> + drm_mode_debug_printmodeline(&crtc->base.mode);
> + }
> }
>
> if (HAS_DDI(dev))
> @@ -9038,12 +9101,14 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>
> list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> base.head) {
> - pipe = 0;
> + pipe = -1;
>
> if (encoder->get_hw_state(encoder, &pipe)) {
> - encoder->base.crtc =
> - dev_priv->pipe_to_crtc_mapping[pipe];
> - } else {
> + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> + if (crtc->mode_valid && encoder->get_mode_flags)
> + crtc->base.mode.flags |= encoder->get_mode_flags(encoder);
> + encoder->base.crtc = &crtc->base;
> + } else
missing {
> encoder->base.crtc = NULL;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1b76b04..c82aed3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1409,6 +1409,27 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_dp_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_dp->output_reg);
> +
> + if (tmp & DP_SYNC_HS_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & DP_SYNC_VS_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_dp(struct intel_encoder *encoder)
> {
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> @@ -2947,6 +2968,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> intel_encoder->disable = intel_disable_dp;
> intel_encoder->post_disable = intel_post_disable_dp;
> intel_encoder->get_hw_state = intel_dp_get_hw_state;
> + intel_encoder->get_mode_flags = intel_dp_get_mode_flags;
>
> intel_dig_port->port = port;
> intel_dig_port->dp.output_reg = output_reg;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 985e9dd..a6c0b25 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -176,6 +176,9 @@ struct intel_encoder {
> * the encoder is active. If the encoder is enabled it also set the pipe
> * it is connected to in the pipe parameter. */
> bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
> + /* Reconstructs the equivalent mode flags for the current hardware
> + * state. */
> + unsigned (*get_mode_flags)(struct intel_encoder *);
> int crtc_mask;
> };
>
> @@ -577,8 +580,8 @@ extern void intel_connector_attach_encoder(struct intel_connector *connector,
> struct intel_encoder *encoder);
> extern struct drm_encoder *intel_best_encoder(struct drm_connector *connector);
>
> -extern struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> - struct drm_crtc *crtc);
> +extern bool intel_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode);
> int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> extern enum transcoder
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 15da995..46b549e 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -129,6 +129,22 @@ static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned
> +intel_dvo_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_dvo *intel_dvo = enc_to_intel_dvo(&encoder->base);
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_dvo->dev.dvo_reg);
> + if (tmp & DVO_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + if (tmp & DVO_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_dvo(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -390,29 +406,26 @@ intel_dvo_get_current_mode(struct drm_connector *connector)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> uint32_t dvo_val = I915_READ(intel_dvo->dev.dvo_reg);
> - struct drm_display_mode *mode = NULL;
> + struct drm_display_mode *fixed_mode = NULL;
>
> /* If the DVO port is active, that'll be the LVDS, so we can pull out
> * its timings to get how the BIOS set up the panel.
> */
> if (dvo_val & DVO_ENABLE) {
> + struct drm_display_mode mode;
> struct drm_crtc *crtc;
> int pipe = (dvo_val & DVO_PIPE_B_SELECT) ? 1 : 0;
>
> crtc = intel_get_crtc_for_pipe(dev, pipe);
> - if (crtc) {
> - mode = intel_crtc_mode_get(dev, crtc);
> - if (mode) {
> - mode->type |= DRM_MODE_TYPE_PREFERRED;
> - if (dvo_val & DVO_HSYNC_ACTIVE_HIGH)
> - mode->flags |= DRM_MODE_FLAG_PHSYNC;
> - if (dvo_val & DVO_VSYNC_ACTIVE_HIGH)
> - mode->flags |= DRM_MODE_FLAG_PVSYNC;
> - }
> + if (intel_crtc_get_mode(crtc, &mode))
> + fixed_mode = drm_mode_duplicate(dev, &mode);
> + if (fixed_mode) {
> + fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> + fixed_mode->flags = intel_dvo_get_mode_flags(&intel_dvo->base);
> }
> }
>
> - return mode;
> + return fixed_mode;
> }
>
> void intel_dvo_init(struct drm_device *dev)
> @@ -441,6 +454,7 @@ void intel_dvo_init(struct drm_device *dev)
> intel_encoder->disable = intel_disable_dvo;
> intel_encoder->enable = intel_enable_dvo;
> intel_encoder->get_hw_state = intel_dvo_get_hw_state;
> + intel_encoder->get_mode_flags = intel_dvo_get_mode_flags;
> intel_connector->get_hw_state = intel_dvo_connector_get_hw_state;
>
> /* Now, try to find a controller */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 5b4efd6..94e3d5d 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -656,6 +656,27 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_hdmi_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_hdmi->sdvox_reg);
> +
> + if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
These sdvo bits are I guess a copy-paste error.
I haven't found any obvious problem with this patch, but I'd wait for a
new version that I can test on ILK. For patches 1-5, taking into account
my comments there:
Reviewed-by: Imre Deak <imre.deak@intel.com>
> +
> + return flags;
> +}
> +
> static void intel_enable_hdmi(struct intel_encoder *encoder)
> {
> struct drm_device *dev = encoder->base.dev;
> @@ -1102,6 +1123,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
> intel_encoder->enable = intel_enable_hdmi;
> intel_encoder->disable = intel_disable_hdmi;
> intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
> + intel_encoder->get_mode_flags = intel_hdmi_get_mode_flags;
>
> intel_encoder->type = INTEL_OUTPUT_HDMI;
> intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 7e4ec63..3da1b2a 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -89,6 +89,26 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_lvds_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 lvds_reg, tmp, flags = 0;
> +
> + if (HAS_PCH_SPLIT(dev))
> + lvds_reg = PCH_LVDS;
> + else
> + lvds_reg = LVDS;
> +
> + tmp = I915_READ(lvds_reg);
> + if (tmp & LVDS_HSYNC_POLARITY)
> + flags |= DRM_MODE_FLAG_NHSYNC;
> + if (tmp & LVDS_VSYNC_POLARITY)
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> /* The LVDS pin pair needs to be on before the DPLLs are enabled.
> * This is an exception to the general rule that mode_set doesn't turn
> * things on.
> @@ -1112,6 +1132,7 @@ bool intel_lvds_init(struct drm_device *dev)
> intel_encoder->pre_pll_enable = intel_pre_pll_enable_lvds;
> intel_encoder->disable = intel_disable_lvds;
> intel_encoder->get_hw_state = intel_lvds_get_hw_state;
> + intel_encoder->get_mode_flags = intel_lvds_get_mode_flags;
> intel_connector->get_hw_state = intel_connector_get_hw_state;
>
> intel_connector_attach_encoder(intel_connector, intel_encoder);
> @@ -1223,11 +1244,15 @@ bool intel_lvds_init(struct drm_device *dev)
> crtc = intel_get_crtc_for_pipe(dev, pipe);
>
> if (crtc && (lvds & LVDS_PORT_EN)) {
> - fixed_mode = intel_crtc_mode_get(dev, crtc);
> + struct drm_display_mode mode;
> +
> + if (intel_crtc_get_mode(crtc, &mode))
> + fixed_mode = drm_mode_duplicate(dev, &mode);
> if (fixed_mode) {
> DRM_DEBUG_KMS("using current (BIOS) mode: ");
> drm_mode_debug_printmodeline(fixed_mode);
> fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> + fixed_mode->flags = intel_lvds_get_mode_flags(intel_encoder);
> goto out;
> }
> }
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index f01063a..918c269 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1250,6 +1250,27 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_sdvo_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_sdvo->sdvo_reg);
> +
> + if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_sdvo(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -2785,6 +2806,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
> intel_encoder->disable = intel_disable_sdvo;
> intel_encoder->enable = intel_enable_sdvo;
> intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
> + if (INTEL_INFO(dev)->gen >= 4)
> + intel_encoder->get_mode_flags = intel_sdvo_get_mode_flags;
>
> /* In default case sdvo lvds is false */
> if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 6/8] drm/i915: Retrieve the current mode upon KMS takeover
2013-02-06 11:10 ` [PATCH 6/8] drm/i915: Retrieve the current mode upon KMS takeover Chris Wilson
2013-02-07 12:58 ` Paulo Zanoni
2013-03-08 15:46 ` Imre Deak
@ 2013-03-08 15:51 ` Imre Deak
2 siblings, 0 replies; 20+ messages in thread
From: Imre Deak @ 2013-03-08 15:51 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, 2013-02-06 at 11:10 +0000, Chris Wilson wrote:
> Read the current hardware state to retrieve the active mode and populate
> our CRTC config if that mode matches our presumptions.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/intel_crt.c | 27 +++++++-
> drivers/gpu/drm/i915/intel_display.c | 119 ++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/intel_dp.c | 22 +++++++
> drivers/gpu/drm/i915/intel_drv.h | 7 +-
> drivers/gpu/drm/i915/intel_dvo.c | 36 ++++++----
> drivers/gpu/drm/i915/intel_hdmi.c | 22 +++++++
> drivers/gpu/drm/i915/intel_lvds.c | 27 +++++++-
> drivers/gpu/drm/i915/intel_sdvo.c | 23 +++++++
> 9 files changed, 242 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6f4afbf..aff1436 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -280,6 +280,8 @@ struct drm_i915_display_funcs {
> void (*update_linetime_wm)(struct drm_device *dev, int pipe,
> struct drm_display_mode *mode);
> void (*modeset_global_resources)(struct drm_device *dev);
> + bool (*crtc_get_mode)(struct drm_crtc *crtc,
> + struct drm_display_mode *mode);
> int (*crtc_mode_set)(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> struct drm_display_mode *adjusted_mode,
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 68e79f3..aa3001a 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -81,6 +81,27 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_crt_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_crt *crt = intel_encoder_to_crt(encoder);
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(crt->adpa_reg);
> +
> + if (tmp & ADPA_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & ADPA_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_crt(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -777,10 +798,12 @@ void intel_crt_init(struct drm_device *dev)
>
> crt->base.disable = intel_disable_crt;
> crt->base.enable = intel_enable_crt;
> - if (HAS_DDI(dev))
> + if (HAS_DDI(dev)) {
> crt->base.get_hw_state = intel_ddi_get_hw_state;
> - else
> + } else {
> crt->base.get_hw_state = intel_crt_get_hw_state;
> + crt->base.get_mode_flags = intel_crt_get_mode_flags;
> + }
> intel_connector->get_hw_state = intel_connector_get_hw_state;
>
> drm_encoder_helper_add(&crt->base.base, &crt_encoder_funcs);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7c4c7d5..43e226d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6600,11 +6600,12 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> }
>
> /* Returns the clock of the currently programmed mode of the given pipe. */
> -static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
> +static int i9xx_crtc_clock_get(struct drm_crtc *crtc)
> {
> + struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pipe = intel_crtc->pipe;
> + enum pipe pipe = intel_crtc->pipe;
> u32 dpll = I915_READ(DPLL(pipe));
> u32 fp;
> intel_clock_t clock;
> @@ -6687,35 +6688,84 @@ static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
> }
>
> /** Returns the currently programmed mode of the given pipe. */
> -struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> - struct drm_crtc *crtc)
> +static bool i9xx_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> - struct drm_display_mode *mode;
> - int htot = I915_READ(HTOTAL(cpu_transcoder));
> - int hsync = I915_READ(HSYNC(cpu_transcoder));
> - int vtot = I915_READ(VTOTAL(cpu_transcoder));
> - int vsync = I915_READ(VSYNC(cpu_transcoder));
> + u32 tmp;
>
> - mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> - if (!mode)
> - return NULL;
> + memset(mode, 0, sizeof(*mode));
> +
> + tmp = I915_READ(HTOTAL(cpu_transcoder));
> + mode->hdisplay = (tmp & 0xffff) + 1;
> + mode->htotal = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(HSYNC(cpu_transcoder));
> + mode->hsync_start = (tmp & 0xffff) + 1;
> + mode->hsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(VTOTAL(cpu_transcoder));
> + mode->vdisplay = (tmp & 0xffff) + 1;
> + mode->vtotal = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(VSYNC(cpu_transcoder));
> + mode->vsync_start = (tmp & 0xffff) + 1;
> + mode->vsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + mode->clock = i9xx_crtc_clock_get(crtc);
> +
> + drm_mode_set_name(mode);
> +
> + return true;
> +}
> +
> +static bool ironlake_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> + u32 tmp;
> +
> + memset(mode, 0, sizeof(*mode));
> +
> + tmp = I915_READ(HTOTAL(cpu_transcoder));
> + mode->hdisplay = (tmp & 0xffff) + 1;
> + mode->htotal = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(HSYNC(cpu_transcoder));
> + mode->hsync_start = (tmp & 0xffff) + 1;
> + mode->hsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + tmp = I915_READ(VTOTAL(cpu_transcoder));
> + mode->vdisplay = (tmp & 0xffff) + 1;
> + mode->vtotal = ((tmp & 0xffff0000) >> 16) + 1;
>
> - mode->clock = intel_crtc_clock_get(dev, crtc);
> - mode->hdisplay = (htot & 0xffff) + 1;
> - mode->htotal = ((htot & 0xffff0000) >> 16) + 1;
> - mode->hsync_start = (hsync & 0xffff) + 1;
> - mode->hsync_end = ((hsync & 0xffff0000) >> 16) + 1;
> - mode->vdisplay = (vtot & 0xffff) + 1;
> - mode->vtotal = ((vtot & 0xffff0000) >> 16) + 1;
> - mode->vsync_start = (vsync & 0xffff) + 1;
> - mode->vsync_end = ((vsync & 0xffff0000) >> 16) + 1;
> + tmp = I915_READ(VSYNC(cpu_transcoder));
> + mode->vsync_start = (tmp & 0xffff) + 1;
> + mode->vsync_end = ((tmp & 0xffff0000) >> 16) + 1;
> +
> + //mode->clock = i9xx_crtc_clock_get(crtc);
> + //mode->clock = 69300;
>
> drm_mode_set_name(mode);
>
> - return mode;
> + return false; /* XXX mode->clock unset */
> +}
> +
> +static __maybe_unused bool no_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + return false;
> +}
> +
> +bool intel_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> +{
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + return dev_priv->display.crtc_get_mode(crtc, mode);
> }
>
> static void intel_increase_pllclock(struct drm_crtc *crtc)
> @@ -8478,20 +8528,25 @@ static void intel_init_display(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> + dev_priv->display.crtc_get_mode = no_crtc_get_mode;
> +
> /* We always want a DPMS function */
> if (HAS_DDI(dev)) {
> + dev_priv->display.crtc_get_mode = ironlake_crtc_get_mode;
> dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
> dev_priv->display.crtc_enable = haswell_crtc_enable;
> dev_priv->display.crtc_disable = haswell_crtc_disable;
> dev_priv->display.off = haswell_crtc_off;
> dev_priv->display.update_plane = ironlake_update_plane;
> } else if (HAS_PCH_SPLIT(dev)) {
> + dev_priv->display.crtc_get_mode = ironlake_crtc_get_mode;
> dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
> dev_priv->display.crtc_enable = ironlake_crtc_enable;
> dev_priv->display.crtc_disable = ironlake_crtc_disable;
> dev_priv->display.off = ironlake_crtc_off;
> dev_priv->display.update_plane = ironlake_update_plane;
> } else {
> + dev_priv->display.crtc_get_mode = i9xx_crtc_get_mode;
> dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
> dev_priv->display.crtc_enable = i9xx_crtc_enable;
> dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -9031,6 +9086,14 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> crtc->base.base.id,
> crtc->active ? "enabled" : "disabled");
> +
> +
> + if (crtc->base.enabled)
> + crtc->mode_valid = intel_crtc_get_mode(&crtc->base, &crtc->base.mode);
Ok, this explains the invalid mode I got and so you can ignore my
comment about it in the previous patch. Well, also for ILK there is
still the missing clock query in ironlake_crtc_get_mode(), so I couldn't
test this.
> + if (crtc->mode_valid) {
> + DRM_DEBUG_KMS("found active mode: ");
> + drm_mode_debug_printmodeline(&crtc->base.mode);
> + }
> }
>
> if (HAS_DDI(dev))
> @@ -9038,12 +9101,14 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>
> list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> base.head) {
> - pipe = 0;
> + pipe = -1;
>
> if (encoder->get_hw_state(encoder, &pipe)) {
> - encoder->base.crtc =
> - dev_priv->pipe_to_crtc_mapping[pipe];
> - } else {
> + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> + if (crtc->mode_valid && encoder->get_mode_flags)
> + crtc->base.mode.flags |= encoder->get_mode_flags(encoder);
> + encoder->base.crtc = &crtc->base;
> + } else
missing {
> encoder->base.crtc = NULL;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1b76b04..c82aed3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1409,6 +1409,27 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_dp_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_dp->output_reg);
> +
> + if (tmp & DP_SYNC_HS_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & DP_SYNC_VS_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_dp(struct intel_encoder *encoder)
> {
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> @@ -2947,6 +2968,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> intel_encoder->disable = intel_disable_dp;
> intel_encoder->post_disable = intel_post_disable_dp;
> intel_encoder->get_hw_state = intel_dp_get_hw_state;
> + intel_encoder->get_mode_flags = intel_dp_get_mode_flags;
>
> intel_dig_port->port = port;
> intel_dig_port->dp.output_reg = output_reg;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 985e9dd..a6c0b25 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -176,6 +176,9 @@ struct intel_encoder {
> * the encoder is active. If the encoder is enabled it also set the pipe
> * it is connected to in the pipe parameter. */
> bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
> + /* Reconstructs the equivalent mode flags for the current hardware
> + * state. */
> + unsigned (*get_mode_flags)(struct intel_encoder *);
> int crtc_mask;
> };
>
> @@ -577,8 +580,8 @@ extern void intel_connector_attach_encoder(struct intel_connector *connector,
> struct intel_encoder *encoder);
> extern struct drm_encoder *intel_best_encoder(struct drm_connector *connector);
>
> -extern struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
> - struct drm_crtc *crtc);
> +extern bool intel_crtc_get_mode(struct drm_crtc *crtc,
> + struct drm_display_mode *mode);
> int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> extern enum transcoder
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 15da995..46b549e 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -129,6 +129,22 @@ static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned
> +intel_dvo_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + struct intel_dvo *intel_dvo = enc_to_intel_dvo(&encoder->base);
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_dvo->dev.dvo_reg);
> + if (tmp & DVO_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + if (tmp & DVO_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_dvo(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -390,29 +406,26 @@ intel_dvo_get_current_mode(struct drm_connector *connector)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
> uint32_t dvo_val = I915_READ(intel_dvo->dev.dvo_reg);
> - struct drm_display_mode *mode = NULL;
> + struct drm_display_mode *fixed_mode = NULL;
>
> /* If the DVO port is active, that'll be the LVDS, so we can pull out
> * its timings to get how the BIOS set up the panel.
> */
> if (dvo_val & DVO_ENABLE) {
> + struct drm_display_mode mode;
> struct drm_crtc *crtc;
> int pipe = (dvo_val & DVO_PIPE_B_SELECT) ? 1 : 0;
>
> crtc = intel_get_crtc_for_pipe(dev, pipe);
> - if (crtc) {
> - mode = intel_crtc_mode_get(dev, crtc);
> - if (mode) {
> - mode->type |= DRM_MODE_TYPE_PREFERRED;
> - if (dvo_val & DVO_HSYNC_ACTIVE_HIGH)
> - mode->flags |= DRM_MODE_FLAG_PHSYNC;
> - if (dvo_val & DVO_VSYNC_ACTIVE_HIGH)
> - mode->flags |= DRM_MODE_FLAG_PVSYNC;
> - }
> + if (intel_crtc_get_mode(crtc, &mode))
> + fixed_mode = drm_mode_duplicate(dev, &mode);
> + if (fixed_mode) {
> + fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> + fixed_mode->flags = intel_dvo_get_mode_flags(&intel_dvo->base);
> }
> }
>
> - return mode;
> + return fixed_mode;
> }
>
> void intel_dvo_init(struct drm_device *dev)
> @@ -441,6 +454,7 @@ void intel_dvo_init(struct drm_device *dev)
> intel_encoder->disable = intel_disable_dvo;
> intel_encoder->enable = intel_enable_dvo;
> intel_encoder->get_hw_state = intel_dvo_get_hw_state;
> + intel_encoder->get_mode_flags = intel_dvo_get_mode_flags;
> intel_connector->get_hw_state = intel_dvo_connector_get_hw_state;
>
> /* Now, try to find a controller */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 5b4efd6..94e3d5d 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -656,6 +656,27 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_hdmi_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_hdmi->sdvox_reg);
> +
> + if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
These SDVO bits are I guess a copy-paste error.
I haven't found any obvious problem with this patch, but I'd wait for a
new version that I can test on ILK. For patches 1-5, taking into account
my comments there:
Reviewed-by: Imre Deak <imre.deak@intel.com>
> +
> + return flags;
> +}
> +
> static void intel_enable_hdmi(struct intel_encoder *encoder)
> {
> struct drm_device *dev = encoder->base.dev;
> @@ -1102,6 +1123,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
> intel_encoder->enable = intel_enable_hdmi;
> intel_encoder->disable = intel_disable_hdmi;
> intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
> + intel_encoder->get_mode_flags = intel_hdmi_get_mode_flags;
>
> intel_encoder->type = INTEL_OUTPUT_HDMI;
> intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 7e4ec63..3da1b2a 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -89,6 +89,26 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_lvds_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 lvds_reg, tmp, flags = 0;
> +
> + if (HAS_PCH_SPLIT(dev))
> + lvds_reg = PCH_LVDS;
> + else
> + lvds_reg = LVDS;
> +
> + tmp = I915_READ(lvds_reg);
> + if (tmp & LVDS_HSYNC_POLARITY)
> + flags |= DRM_MODE_FLAG_NHSYNC;
> + if (tmp & LVDS_VSYNC_POLARITY)
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> /* The LVDS pin pair needs to be on before the DPLLs are enabled.
> * This is an exception to the general rule that mode_set doesn't turn
> * things on.
> @@ -1112,6 +1132,7 @@ bool intel_lvds_init(struct drm_device *dev)
> intel_encoder->pre_pll_enable = intel_pre_pll_enable_lvds;
> intel_encoder->disable = intel_disable_lvds;
> intel_encoder->get_hw_state = intel_lvds_get_hw_state;
> + intel_encoder->get_mode_flags = intel_lvds_get_mode_flags;
> intel_connector->get_hw_state = intel_connector_get_hw_state;
>
> intel_connector_attach_encoder(intel_connector, intel_encoder);
> @@ -1223,11 +1244,15 @@ bool intel_lvds_init(struct drm_device *dev)
> crtc = intel_get_crtc_for_pipe(dev, pipe);
>
> if (crtc && (lvds & LVDS_PORT_EN)) {
> - fixed_mode = intel_crtc_mode_get(dev, crtc);
> + struct drm_display_mode mode;
> +
> + if (intel_crtc_get_mode(crtc, &mode))
> + fixed_mode = drm_mode_duplicate(dev, &mode);
> if (fixed_mode) {
> DRM_DEBUG_KMS("using current (BIOS) mode: ");
> drm_mode_debug_printmodeline(fixed_mode);
> fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
> + fixed_mode->flags = intel_lvds_get_mode_flags(intel_encoder);
> goto out;
> }
> }
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index f01063a..918c269 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1250,6 +1250,27 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
> return true;
> }
>
> +static unsigned intel_sdvo_get_mode_flags(struct intel_encoder *encoder)
> +{
> + struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
> + struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> + u32 tmp, flags = 0;
> +
> + tmp = I915_READ(intel_sdvo->sdvo_reg);
> +
> + if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
> + flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return flags;
> +}
> +
> static void intel_disable_sdvo(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -2785,6 +2806,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
> intel_encoder->disable = intel_disable_sdvo;
> intel_encoder->enable = intel_enable_sdvo;
> intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
> + if (INTEL_INFO(dev)->gen >= 4)
> + intel_encoder->get_mode_flags = intel_sdvo_get_mode_flags;
>
> /* In default case sdvo lvds is false */
> if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 7/8] drm/i915: Only preserve the BIOS modes if they are the preferred ones
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
` (4 preceding siblings ...)
2013-02-06 11:10 ` [PATCH 6/8] drm/i915: Retrieve the current mode upon KMS takeover Chris Wilson
@ 2013-02-06 11:10 ` Chris Wilson
2013-02-06 11:10 ` [PATCH 8/8] drm/i915: Validate that the framebuffer accommodates the current mode Chris Wilson
` (3 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2013-02-06 11:10 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
drivers/gpu/drm/i915/intel_dp.c | 1 +
drivers/gpu/drm/i915/intel_drv.h | 8 ++++++++
drivers/gpu/drm/i915/intel_fb.c | 9 +++++++++
drivers/gpu/drm/i915/intel_lvds.c | 1 +
drivers/gpu/drm/i915/intel_panel.c | 10 ++++++++++
6 files changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 43e226d..c1d8200 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9233,6 +9233,15 @@ void intel_connector_attach_encoder(struct intel_connector *connector,
&encoder->base);
}
+bool intel_connector_get_preferred_mode(struct intel_connector *connector,
+ struct drm_display_mode *mode)
+{
+ if (!connector->get_preferred_mode)
+ return false;
+
+ return connector->get_preferred_mode(connector, mode);
+}
+
/*
* set vga decode state - true == enable VGA decode
*/
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c82aed3..c8597d9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2922,6 +2922,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
}
if (is_edp(intel_dp)) {
+ intel_connector->get_preferred_mode = intel_connector_get_panel_fixed_mode;
intel_panel_init(&intel_connector->panel, fixed_mode);
intel_panel_setup_backlight(connector);
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a6c0b25..8bd9bf0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -204,6 +204,9 @@ struct intel_connector {
* and active (i.e. dpms ON state). */
bool (*get_hw_state)(struct intel_connector *);
+ bool (*get_preferred_mode)(struct intel_connector *,
+ struct drm_display_mode *);
+
/* Panel info for eDP and LVDS */
struct intel_panel panel;
@@ -505,6 +508,9 @@ extern int intel_panel_init(struct intel_panel *panel,
struct drm_display_mode *fixed_mode);
extern void intel_panel_fini(struct intel_panel *panel);
+extern bool intel_connector_get_panel_fixed_mode(struct intel_connector *connector,
+ struct drm_display_mode *mode);
+
extern void intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode);
extern void intel_pch_panel_fitting(struct drm_device *dev,
@@ -576,6 +582,8 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
struct intel_digital_port *port);
+extern bool intel_connector_get_preferred_mode(struct intel_connector *connector,
+ struct drm_display_mode *mode);
extern void intel_connector_attach_encoder(struct intel_connector *connector,
struct intel_encoder *encoder);
extern struct drm_encoder *intel_best_encoder(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index ca6c8e6..32bc904 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -237,6 +237,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
for (i = 0; i < fb_helper->connector_count; i++) {
struct drm_connector *connector;
struct drm_encoder *encoder;
+ struct drm_display_mode mode;
connector = fb_helper->connector_info[i]->connector;
if (!enabled[i]) {
@@ -266,6 +267,14 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
return false;
}
+ if (intel_connector_get_preferred_mode(to_intel_connector(connector), &mode) &&
+ !drm_mode_equal(&mode, &encoder->crtc->mode)) {
+ DRM_DEBUG_KMS("connector %s on crtc %d has an non-native mode, aborting\n",
+ drm_get_connector_name(connector),
+ encoder->crtc->base.id);
+ return false;
+ }
+
modes[i] = &encoder->crtc->mode;
crtcs[i] = intel_fb_helper_crtc(fb_helper, encoder->crtc);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 3da1b2a..9f4381e 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1134,6 +1134,7 @@ bool intel_lvds_init(struct drm_device *dev)
intel_encoder->get_hw_state = intel_lvds_get_hw_state;
intel_encoder->get_mode_flags = intel_lvds_get_mode_flags;
intel_connector->get_hw_state = intel_connector_get_hw_state;
+ intel_connector->get_preferred_mode = intel_connector_get_panel_fixed_mode;
intel_connector_attach_encoder(intel_connector, intel_encoder);
intel_encoder->type = INTEL_OUTPUT_LVDS;
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index bee8cb6..212e088 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -35,6 +35,16 @@
#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
+bool intel_connector_get_panel_fixed_mode(struct intel_connector *connector,
+ struct drm_display_mode *mode)
+{
+ if (!connector->panel.fixed_mode)
+ return false;
+
+ *mode = *connector->panel.fixed_mode;
+ return true;
+}
+
void
intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
struct drm_display_mode *adjusted_mode)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 8/8] drm/i915: Validate that the framebuffer accommodates the current mode
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
` (5 preceding siblings ...)
2013-02-06 11:10 ` [PATCH 7/8] drm/i915: Only preserve the BIOS modes if they are the preferred ones Chris Wilson
@ 2013-02-06 11:10 ` Chris Wilson
2013-02-06 12:46 ` [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Daniel Vetter
` (2 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2013-02-06 11:10 UTC (permalink / raw)
To: intel-gfx
As we retrieve the mode from the BIOS it may be constructed using
different assumptions for its configuration, such as utilizing the panel
fitter in a conflicting manner. As such the associated framebuffer may be
insufficient for our setup, and so we need to reject the current mode
and install our own.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c1d8200..f5d4d88 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6441,27 +6441,40 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
return intel_framebuffer_create(dev, &mode_cmd, obj);
}
+static bool
+mode_fits_in_fb(struct drm_display_mode *mode,
+ struct drm_framebuffer *fb)
+{
+ struct drm_i915_gem_object *obj;
+ int min_pitch;
+
+ min_pitch = intel_framebuffer_pitch_for_width(mode->hdisplay,
+ fb->bits_per_pixel);
+ if (fb->pitches[0] < min_pitch)
+ return false;
+
+ obj = to_intel_framebuffer(fb)->obj;
+ if (obj == NULL)
+ return false;
+
+ if (obj->base.size < mode->vdisplay * fb->pitches[0])
+ return false;
+
+ return true;
+}
+
static struct drm_framebuffer *
mode_fits_in_fbdev(struct drm_device *dev,
struct drm_display_mode *mode)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_i915_gem_object *obj;
struct drm_framebuffer *fb;
if (dev_priv->fbdev == NULL)
return NULL;
- obj = dev_priv->fbdev->ifb.obj;
- if (obj == NULL)
- return NULL;
-
fb = &dev_priv->fbdev->ifb.base;
- if (fb->pitches[0] < intel_framebuffer_pitch_for_width(mode->hdisplay,
- fb->bits_per_pixel))
- return NULL;
-
- if (obj->base.size < mode->vdisplay * fb->pitches[0])
+ if (!mode_fits_in_fb(mode, fb))
return NULL;
return fb;
@@ -9090,6 +9103,11 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
if (crtc->base.enabled)
crtc->mode_valid = intel_crtc_get_mode(&crtc->base, &crtc->base.mode);
+
+ if (crtc->base.fb &&
+ !mode_fits_in_fb(&crtc->base.mode, crtc->base.fb))
+ crtc->mode_valid = false;
+
if (crtc->mode_valid) {
DRM_DEBUG_KMS("found active mode: ");
drm_mode_debug_printmodeline(&crtc->base.mode);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
` (6 preceding siblings ...)
2013-02-06 11:10 ` [PATCH 8/8] drm/i915: Validate that the framebuffer accommodates the current mode Chris Wilson
@ 2013-02-06 12:46 ` Daniel Vetter
2013-02-07 12:45 ` Paulo Zanoni
2013-03-07 13:16 ` Imre Deak
9 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2013-02-06 12:46 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, Feb 06, 2013 at 11:10:21AM +0000, Chris Wilson wrote:
> Modifying the clock sources (via the DREF control on the PCH) is a slow
> multi-stage process as we need to let the clocks stabilise between each
> stage. If we are not actually changing the clock sources, then we can
> return early.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
One idea I've been pondering for the refclock stuff is to simply shovel
this into the ->modeset_global_resources callback. That way no clever
tricks are required, and we'd avoid this for all cases where fastboot
works complety. Presumably ofc that the bios didn't set up a config which
does not work.
As a bonus, we could only set up the refclocks the new configuration
actually needs, i.e. filter for encoder->crtc != NULL ... A bit a risky
patch, but might uncover some subtle bugs if we do the refclock adjusting
more often.
Too insane?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++++++---------
> 1 file changed, 61 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d75c6a0..f1dbd01 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4749,7 +4749,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_mode_config *mode_config = &dev->mode_config;
> struct intel_encoder *encoder;
> - u32 temp;
> + u32 val, final;
> bool has_lvds = false;
> bool has_cpu_edp = false;
> bool has_pch_edp = false;
> @@ -4792,70 +4792,109 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> * PCH B stepping, previous chipset stepping should be
> * ignoring this setting.
> */
> - temp = I915_READ(PCH_DREF_CONTROL);
> + val = I915_READ(PCH_DREF_CONTROL);
> +
> + /* As we must carefully and slowly disable/enable each source in turn,
> + * compute the final state we want first and check if we need to
> + * make any changes at all.
> + */
> + final = val;
> + final &= ~DREF_NONSPREAD_SOURCE_MASK;
> + if (has_ck505)
> + final |= DREF_NONSPREAD_CK505_ENABLE;
> + else
> + final |= DREF_NONSPREAD_SOURCE_ENABLE;
> +
> + final &= ~DREF_SSC_SOURCE_MASK;
> + final &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> + final &= ~DREF_SSC1_ENABLE;
> +
> + if (has_panel) {
> + final |= DREF_SSC_SOURCE_ENABLE;
> +
> + if (intel_panel_use_ssc(dev_priv) && can_ssc)
> + final |= DREF_SSC1_ENABLE;
> +
> + if (has_cpu_edp) {
> + if (intel_panel_use_ssc(dev_priv) && can_ssc)
> + final |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> + else
> + final |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> + } else
> + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + } else {
> + final |= DREF_SSC_SOURCE_DISABLE;
> + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + }
> +
> + if (final == val)
> + return;
> +
> /* Always enable nonspread source */
> - temp &= ~DREF_NONSPREAD_SOURCE_MASK;
> + val &= ~DREF_NONSPREAD_SOURCE_MASK;
>
> if (has_ck505)
> - temp |= DREF_NONSPREAD_CK505_ENABLE;
> + val |= DREF_NONSPREAD_CK505_ENABLE;
> else
> - temp |= DREF_NONSPREAD_SOURCE_ENABLE;
> + val |= DREF_NONSPREAD_SOURCE_ENABLE;
>
> if (has_panel) {
> - temp &= ~DREF_SSC_SOURCE_MASK;
> - temp |= DREF_SSC_SOURCE_ENABLE;
> + val &= ~DREF_SSC_SOURCE_MASK;
> + val |= DREF_SSC_SOURCE_ENABLE;
>
> /* SSC must be turned on before enabling the CPU output */
> if (intel_panel_use_ssc(dev_priv) && can_ssc) {
> DRM_DEBUG_KMS("Using SSC on panel\n");
> - temp |= DREF_SSC1_ENABLE;
> + val |= DREF_SSC1_ENABLE;
> } else
> - temp &= ~DREF_SSC1_ENABLE;
> + val &= ~DREF_SSC1_ENABLE;
>
> /* Get SSC going before enabling the outputs */
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
>
> - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
>
> /* Enable CPU source on CPU attached eDP */
> if (has_cpu_edp) {
> if (intel_panel_use_ssc(dev_priv) && can_ssc) {
> DRM_DEBUG_KMS("Using SSC on eDP\n");
> - temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> + val |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> }
> else
> - temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> + val |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> } else
> - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
>
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
> } else {
> DRM_DEBUG_KMS("Disabling SSC entirely\n");
>
> - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
>
> /* Turn off CPU output */
> - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
>
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
>
> /* Turn off the SSC source */
> - temp &= ~DREF_SSC_SOURCE_MASK;
> - temp |= DREF_SSC_SOURCE_DISABLE;
> + val &= ~DREF_SSC_SOURCE_MASK;
> + val |= DREF_SSC_SOURCE_DISABLE;
>
> /* Turn off SSC1 */
> - temp &= ~ DREF_SSC1_ENABLE;
> + val &= ~ DREF_SSC1_ENABLE;
>
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
> }
> +
> + BUG_ON(val != final);
> }
>
> /* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
` (7 preceding siblings ...)
2013-02-06 12:46 ` [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Daniel Vetter
@ 2013-02-07 12:45 ` Paulo Zanoni
2013-02-07 13:21 ` Daniel Vetter
2013-03-07 13:16 ` Imre Deak
9 siblings, 1 reply; 20+ messages in thread
From: Paulo Zanoni @ 2013-02-07 12:45 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
Hi
2013/2/6 Chris Wilson <chris@chris-wilson.co.uk>:
> Modifying the clock sources (via the DREF control on the PCH) is a slow
> multi-stage process as we need to let the clocks stabilise between each
> stage. If we are not actually changing the clock sources, then we can
> return early.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
The patch looks correct, so:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
But this patch only saves 400us. I thought this was on Daniel's "don't
care" range (e.g., on "drm/i915: don't send DP "idle" pattern before
"normal" on HSW PORT_A" I was asked to keep a potentially bigger delay
that's not needed at all).
> ---
> drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++++++---------
> 1 file changed, 61 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d75c6a0..f1dbd01 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4749,7 +4749,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_mode_config *mode_config = &dev->mode_config;
> struct intel_encoder *encoder;
> - u32 temp;
> + u32 val, final;
> bool has_lvds = false;
> bool has_cpu_edp = false;
> bool has_pch_edp = false;
> @@ -4792,70 +4792,109 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> * PCH B stepping, previous chipset stepping should be
> * ignoring this setting.
> */
> - temp = I915_READ(PCH_DREF_CONTROL);
> + val = I915_READ(PCH_DREF_CONTROL);
> +
> + /* As we must carefully and slowly disable/enable each source in turn,
> + * compute the final state we want first and check if we need to
> + * make any changes at all.
> + */
> + final = val;
> + final &= ~DREF_NONSPREAD_SOURCE_MASK;
> + if (has_ck505)
> + final |= DREF_NONSPREAD_CK505_ENABLE;
> + else
> + final |= DREF_NONSPREAD_SOURCE_ENABLE;
> +
> + final &= ~DREF_SSC_SOURCE_MASK;
> + final &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> + final &= ~DREF_SSC1_ENABLE;
> +
> + if (has_panel) {
> + final |= DREF_SSC_SOURCE_ENABLE;
> +
> + if (intel_panel_use_ssc(dev_priv) && can_ssc)
> + final |= DREF_SSC1_ENABLE;
> +
> + if (has_cpu_edp) {
> + if (intel_panel_use_ssc(dev_priv) && can_ssc)
> + final |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> + else
> + final |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> + } else
> + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + } else {
> + final |= DREF_SSC_SOURCE_DISABLE;
> + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + }
> +
> + if (final == val)
> + return;
> +
> /* Always enable nonspread source */
> - temp &= ~DREF_NONSPREAD_SOURCE_MASK;
> + val &= ~DREF_NONSPREAD_SOURCE_MASK;
>
> if (has_ck505)
> - temp |= DREF_NONSPREAD_CK505_ENABLE;
> + val |= DREF_NONSPREAD_CK505_ENABLE;
> else
> - temp |= DREF_NONSPREAD_SOURCE_ENABLE;
> + val |= DREF_NONSPREAD_SOURCE_ENABLE;
>
> if (has_panel) {
> - temp &= ~DREF_SSC_SOURCE_MASK;
> - temp |= DREF_SSC_SOURCE_ENABLE;
> + val &= ~DREF_SSC_SOURCE_MASK;
> + val |= DREF_SSC_SOURCE_ENABLE;
>
> /* SSC must be turned on before enabling the CPU output */
> if (intel_panel_use_ssc(dev_priv) && can_ssc) {
> DRM_DEBUG_KMS("Using SSC on panel\n");
> - temp |= DREF_SSC1_ENABLE;
> + val |= DREF_SSC1_ENABLE;
> } else
> - temp &= ~DREF_SSC1_ENABLE;
> + val &= ~DREF_SSC1_ENABLE;
>
> /* Get SSC going before enabling the outputs */
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
>
> - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
>
> /* Enable CPU source on CPU attached eDP */
> if (has_cpu_edp) {
> if (intel_panel_use_ssc(dev_priv) && can_ssc) {
> DRM_DEBUG_KMS("Using SSC on eDP\n");
> - temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> + val |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> }
> else
> - temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> + val |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> } else
> - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
>
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
> } else {
> DRM_DEBUG_KMS("Disabling SSC entirely\n");
>
> - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
>
> /* Turn off CPU output */
> - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
>
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
>
> /* Turn off the SSC source */
> - temp &= ~DREF_SSC_SOURCE_MASK;
> - temp |= DREF_SSC_SOURCE_DISABLE;
> + val &= ~DREF_SSC_SOURCE_MASK;
> + val |= DREF_SSC_SOURCE_DISABLE;
>
> /* Turn off SSC1 */
> - temp &= ~ DREF_SSC1_ENABLE;
> + val &= ~ DREF_SSC1_ENABLE;
>
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
> }
> +
> + BUG_ON(val != final);
> }
>
> /* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources
2013-02-07 12:45 ` Paulo Zanoni
@ 2013-02-07 13:21 ` Daniel Vetter
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2013-02-07 13:21 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Thu, Feb 7, 2013 at 1:45 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> 2013/2/6 Chris Wilson <chris@chris-wilson.co.uk>:
>> Modifying the clock sources (via the DREF control on the PCH) is a slow
>> multi-stage process as we need to let the clocks stabilise between each
>> stage. If we are not actually changing the clock sources, then we can
>> return early.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> The patch looks correct, so:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> But this patch only saves 400us. I thought this was on Daniel's "don't
> care" range (e.g., on "drm/i915: don't send DP "idle" pattern before
> "normal" on HSW PORT_A" I was asked to keep a potentially bigger delay
> that's not needed at all).
For modeset operations it's on my don't care list, since that'll take
much longer anyway. But this tries to avoid it in boot-up, where
fastboot aims to completely avoid the modeset. And I'm toying around
with pushing all the edid reading to workqueues. So this could very
much be in the "I care" bucket ;-)
Hence also my proposal to simply push this into the modeset sequence
instead of making the code more complicated ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources
2013-02-06 11:10 [PATCH 1/8] drm/i915: Skip modifying PCH DREF if not changing clock sources Chris Wilson
` (8 preceding siblings ...)
2013-02-07 12:45 ` Paulo Zanoni
@ 2013-03-07 13:16 ` Imre Deak
9 siblings, 0 replies; 20+ messages in thread
From: Imre Deak @ 2013-03-07 13:16 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, 2013-02-06 at 11:10 +0000, Chris Wilson wrote:
> Modifying the clock sources (via the DREF control on the PCH) is a slow
> multi-stage process as we need to let the clocks stabilise between each
> stage. If we are not actually changing the clock sources, then we can
> return early.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++++++---------
> 1 file changed, 61 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d75c6a0..f1dbd01 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4749,7 +4749,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_mode_config *mode_config = &dev->mode_config;
> struct intel_encoder *encoder;
> - u32 temp;
> + u32 val, final;
> bool has_lvds = false;
> bool has_cpu_edp = false;
> bool has_pch_edp = false;
> @@ -4792,70 +4792,109 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
> * PCH B stepping, previous chipset stepping should be
> * ignoring this setting.
> */
> - temp = I915_READ(PCH_DREF_CONTROL);
> + val = I915_READ(PCH_DREF_CONTROL);
> +
> + /* As we must carefully and slowly disable/enable each source in turn,
> + * compute the final state we want first and check if we need to
> + * make any changes at all.
> + */
> + final = val;
> + final &= ~DREF_NONSPREAD_SOURCE_MASK;
> + if (has_ck505)
> + final |= DREF_NONSPREAD_CK505_ENABLE;
> + else
> + final |= DREF_NONSPREAD_SOURCE_ENABLE;
> +
> + final &= ~DREF_SSC_SOURCE_MASK;
> + final &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> + final &= ~DREF_SSC1_ENABLE;
> +
> + if (has_panel) {
> + final |= DREF_SSC_SOURCE_ENABLE;
> +
> + if (intel_panel_use_ssc(dev_priv) && can_ssc)
> + final |= DREF_SSC1_ENABLE;
> +
> + if (has_cpu_edp) {
> + if (intel_panel_use_ssc(dev_priv) && can_ssc)
> + final |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> + else
> + final |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> + } else
> + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + } else {
> + final |= DREF_SSC_SOURCE_DISABLE;
> + final |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + }
> +
> + if (final == val)
> + return;
> +
Nitpick: this would be clearer in a seperate function with a
'check_only' param, then the below could be removed.
> /* Always enable nonspread source */
> - temp &= ~DREF_NONSPREAD_SOURCE_MASK;
> + val &= ~DREF_NONSPREAD_SOURCE_MASK;
>
> if (has_ck505)
> - temp |= DREF_NONSPREAD_CK505_ENABLE;
> + val |= DREF_NONSPREAD_CK505_ENABLE;
> else
> - temp |= DREF_NONSPREAD_SOURCE_ENABLE;
> + val |= DREF_NONSPREAD_SOURCE_ENABLE;
>
> if (has_panel) {
> - temp &= ~DREF_SSC_SOURCE_MASK;
> - temp |= DREF_SSC_SOURCE_ENABLE;
> + val &= ~DREF_SSC_SOURCE_MASK;
> + val |= DREF_SSC_SOURCE_ENABLE;
>
> /* SSC must be turned on before enabling the CPU output */
> if (intel_panel_use_ssc(dev_priv) && can_ssc) {
> DRM_DEBUG_KMS("Using SSC on panel\n");
> - temp |= DREF_SSC1_ENABLE;
> + val |= DREF_SSC1_ENABLE;
> } else
> - temp &= ~DREF_SSC1_ENABLE;
> + val &= ~DREF_SSC1_ENABLE;
>
> /* Get SSC going before enabling the outputs */
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
>
> - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
>
> /* Enable CPU source on CPU attached eDP */
> if (has_cpu_edp) {
> if (intel_panel_use_ssc(dev_priv) && can_ssc) {
> DRM_DEBUG_KMS("Using SSC on eDP\n");
> - temp |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> + val |= DREF_CPU_SOURCE_OUTPUT_DOWNSPREAD;
> }
> else
> - temp |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> + val |= DREF_CPU_SOURCE_OUTPUT_NONSPREAD;
> } else
> - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
>
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
> } else {
> DRM_DEBUG_KMS("Disabling SSC entirely\n");
>
> - temp &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
> + val &= ~DREF_CPU_SOURCE_OUTPUT_MASK;
>
> /* Turn off CPU output */
> - temp |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
> + val |= DREF_CPU_SOURCE_OUTPUT_DISABLE;
>
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
>
> /* Turn off the SSC source */
> - temp &= ~DREF_SSC_SOURCE_MASK;
> - temp |= DREF_SSC_SOURCE_DISABLE;
> + val &= ~DREF_SSC_SOURCE_MASK;
> + val |= DREF_SSC_SOURCE_DISABLE;
>
> /* Turn off SSC1 */
> - temp &= ~ DREF_SSC1_ENABLE;
> + val &= ~ DREF_SSC1_ENABLE;
>
> - I915_WRITE(PCH_DREF_CONTROL, temp);
> + I915_WRITE(PCH_DREF_CONTROL, val);
> POSTING_READ(PCH_DREF_CONTROL);
> udelay(200);
> }
> +
> + BUG_ON(val != final);
> }
>
> /* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
^ permalink raw reply [flat|nested] 20+ messages in thread