Re: [Intel-gfx] [PATCH] drm/i915: Remove nested work in gpu error handling
On Wed, Jan 28, 2015 at 05:03:14PM +0200, Mika Kuoppala wrote: > @@ -2616,6 +2612,9 @@ void i915_handle_error(struct drm_device *dev, bool > wedged, > va_list args; > char error_msg[80]; > > + if (WARN_ON(mutex_is_locked(&dev_priv->dev->struct_mutex))) > + return; > + Oops, sorry, I should have realised this was wrong earlier. The mutex breaking occurs later in i915_handle_error. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Enable asynchronous nuclear flips
On Sat, Jan 31, 2015 at 12:07:56PM -0800, Matt Roper wrote:
> On Sat, Jan 31, 2015 at 10:30:29AM +0100, Daniel Vetter wrote:
> > On Fri, Jan 30, 2015 at 04:22:38PM -0800, Matt Roper wrote:
> > > The initial i915 nuclear pageflip support rejected asynchronous updates.
> > > Allow all work after we swap in the new state structures to run
> > > asynchronously. We also need to start sending completion events to
> > > userspace if they were requested.
> > >
> > > Signed-off-by: Matt Roper
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 3 +
> > > drivers/gpu/drm/i915/intel_atomic.c | 162
> > > +++-
> > > drivers/gpu/drm/i915/intel_drv.h| 8 ++
> > > 3 files changed, 150 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 8fad702..c7a520a 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1777,6 +1777,9 @@ struct drm_i915_private {
> > > struct drm_crtc *pipe_to_crtc_mapping[I915_MAX_PIPES];
> > > wait_queue_head_t pending_flip_queue;
> > >
> > > + /* CRTC mask of pending atomic flips */
> > > + uint32_t pending_atomic;
> > > +
> > > #ifdef CONFIG_DEBUG_FS
> > > struct intel_pipe_crc pipe_crc[I915_MAX_PIPES];
> > > #endif
> > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > > b/drivers/gpu/drm/i915/intel_atomic.c
> > > index 19a9dd5..5dd7897 100644
> > > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > > @@ -76,6 +76,8 @@ int intel_atomic_check(struct drm_device *dev,
> > > state->allow_modeset = false;
> > > for (i = 0; i < ncrtcs; i++) {
> > > struct intel_crtc *crtc = to_intel_crtc(state->crtcs[i]);
> > > + if (crtc)
> > > + state->crtc_states[i]->enable = crtc->active;
> > > if (crtc && crtc->pipe != nuclear_pipe)
> > > not_nuclear = true;
> > > }
> > > @@ -96,6 +98,87 @@ int intel_atomic_check(struct drm_device *dev,
> > > }
> > >
> > >
> > > +/*
> > > + * Wait until CRTC's have no pending flip, then atomically mark those
> > > CRTC's
> > > + * as busy.
> > > + */
> > > +static int wait_for_pending_flip(uint32_t crtc_mask,
> > > + struct intel_pending_atomic *commit)
> > > +{
> > > + struct drm_i915_private *dev_priv = commit->dev->dev_private;
> > > + int ret;
> > > +
> > > + spin_lock_irq(&dev_priv->pending_flip_queue.lock);
> > > + ret = wait_event_interruptible_locked(dev_priv->pending_flip_queue,
> > > + !(dev_priv->pending_atomic &
> > > crtc_mask));
> > > + if (ret == 0)
> > > + dev_priv->pending_atomic |= crtc_mask;
> > > + spin_unlock_irq(&dev_priv->pending_flip_queue.lock);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +/* Finish pending flip operation on specified CRTC's */
> > > +static void flip_completion(struct intel_pending_atomic *commit)
> > > +{
> > > + struct drm_i915_private *dev_priv = commit->dev->dev_private;
> > > +
> > > + spin_lock_irq(&dev_priv->pending_flip_queue.lock);
> > > + dev_priv->pending_atomic &= ~commit->crtc_mask;
> > > + wake_up_all_locked(&dev_priv->pending_flip_queue);
> > > + spin_unlock_irq(&dev_priv->pending_flip_queue.lock);
> > > +}
> > > +
> > > +/*
> > > + * Finish an atomic commit. The work here can be performed
> > > asynchronously
> > > + * if desired. The new state has already been applied to the DRM objects
> > > + * and no modeset locks are needed.
> > > + */
> > > +static void finish_atomic_commit(struct work_struct *work)
> > > +{
> > > + struct intel_pending_atomic *commit =
> > > + container_of(work, struct intel_pending_atomic, work);
> > > + struct drm_device *dev = commit->dev;
> > > + struct drm_crtc *crtc;
> > > + struct drm_atomic_state *state = commit->state;
> > > + int i;
> > > +
> > > + /*
> > > + * FIXME: The proper sequence here will eventually be:
> > > + *
> > > + * drm_atomic_helper_commit_pre_planes(dev, state);
> > > + * drm_atomic_helper_commit_planes(dev, state);
> > > + * drm_atomic_helper_commit_post_planes(dev, state);
> > > + * drm_atomic_helper_wait_for_vblanks(dev, state);
> > > + * drm_atomic_helper_cleanup_planes(dev, state);
> > > + * drm_atomic_state_free(state);
> > > + *
> > > + * once we have full atomic modeset. For now, just manually update
> > > + * plane states to avoid clobbering good states with dummy states
> > > + * while nuclear pageflipping.
> > > + */
> > > + drm_atomic_helper_commit_planes(dev, state);
> > > + drm_atomic_helper_wait_for_vblanks(dev, state);
> > > +
> > > + /* Send CRTC completion events. */
> > > + for (i = 0; i < dev->mode_config.num_crtc; i++) {
> > > + crtc = state->crtcs[i];
> > > + if (crtc && crtc->state->event) {
> > > + spin_lock_irq(&dev->event_lock);
> > > + drm_send_vblank_event(dev, to_intel_crtc(crtc)-
[Intel-gfx] Kernel 3.19rc6 flooding intel_check_page_flip warnings when using compton
Dear maintainers, On an Asus Zenbook UX32VD laptop with an i5-3317U CPU and Intel HD4000 graphics, I'm experiencing the following with the latest 3.19rc6 mainline kernel (built from the Arch Linux AUR package: https://aur.archlinux.org/packages/linux-mainline/ ). The problem is related to the compton compositor: https://github.com/chjj/compton Booting to X goes very normally. Then, I start compton with the command `compton --backend glx`. As soon as I do that, the dmesg log starts getting filled with these warnings. This is a snippet from my journalctl log: 1 Feb 02 00:06:44 stroemsoe kernel: WARNING: CPU: 0 PID: 273 at drivers/gpu/drm/i915/intel_display.c:9705 intel_check_page_flip+0xa2/0xf0 [i915]() 1421 Feb 02 00:06:44 stroemsoe kernel: WARN_ON(!in_irq()) 1 Feb 02 00:06:44 stroemsoe kernel: Modules linked in: 2 Feb 02 00:06:44 stroemsoe kernel: joydev mousedev msr uvcvideo rtsx_usb_sdmmc videobuf2_vmalloc rtsx_usb_ms videobuf2_memops mmc_core memstick videobuf2_core n ls_iso8859_1 rtsx_usb v4l2_common nls_cp437 videodev vfat fat coretemp intel_rapl media iosf_mbi x86_pkg_temp_thermal intel_powerclamp arc4 kvm_intel kvm iwldvm crct10dif_pclmul crc32_pclmul iTCO_wdt mac80211 iTCO_vendor_support crc32c_intel ghash_clmulni_intel snd_hda_codec_hdmi nouveau aesni_intel i915 aes_x86_64 snd _hda_codec_realtek glue_helper lrw gf128mul ablk_helper snd_hda_codec_generic cryptd iwlwifi pcspkr evdev mac_hid snd_hda_intel psmouse serio_raw snd_hda_contro ller snd_hda_codec snd_hwdep mxm_wmi snd_pcm ttm snd_timer cfg80211 snd lpc_ich soundcore mfd_core i2c_i801 fan int3403_thermal button i2c_algo_bit mei_me drm_k ms_helper mei drm battery tpm_tis 3 Feb 02 00:06:44 stroemsoe kernel: thermal tpm shpchp ac int3402_thermal i2c_core intel_gtt int3400_thermal acpi_thermal_rel processor sch_fq_codel overlay ext4 crc16 mbcache jbd2 sd_mod atkbd libps2 ahci libahci libata ehci_pci xhci_pci ehci_hcd xhci_hcd scsi_mod usbcore usb_common i8042 serio asus_nb_wmi asus_wmi hwm on video rfkill sparse_keymap wmi led_class 4 Feb 02 00:06:44 stroemsoe kernel: CPU: 0 PID: 273 Comm: irq/32-i915 Tainted: GW 3.19.0-1-mainline #1 5 Feb 02 00:06:44 stroemsoe kernel: Hardware name: ASUSTeK COMPUTER INC. UX32VD/UX32VD, BIOS UX32VD.213 11/16/2012 6 Feb 02 00:06:44 stroemsoe kernel: b983161a 8800c88afc78 8153537c 7 Feb 02 00:06:44 stroemsoe kernel: 8800c88afcd0 8800c88afcb8 81075a9a 8 Feb 02 00:06:44 stroemsoe kernel: 8800c88afcb8 8800c7d99000 8800c8e05000 8800c8e05000 9 Feb 02 00:06:44 stroemsoe kernel: Call Trace: 10 Feb 02 00:06:44 stroemsoe kernel: [] dump_stack+0x4c/0x6e 11 Feb 02 00:06:44 stroemsoe kernel: [] warn_slowpath_common+0x8a/0xc0 12 Feb 02 00:06:44 stroemsoe kernel: [] warn_slowpath_fmt+0x55/0x70 13 Feb 02 00:06:44 stroemsoe kernel: [] intel_check_page_flip+0xa2/0xf0 [i915] 14 Feb 02 00:06:44 stroemsoe kernel: [] ironlake_irq_handler+0x428/0x1000 [i915] 15 Feb 02 00:06:44 stroemsoe kernel: [] ? do_set_cpus_allowed+0x4a/0x60 16 Feb 02 00:06:44 stroemsoe kernel: [] ? irq_thread_fn+0x50/0x50 17 Feb 02 00:06:44 stroemsoe kernel: [] irq_forced_thread_fn+0x2d/0x70 18 Feb 02 00:06:44 stroemsoe kernel: [] irq_thread+0x157/0x180 19 Feb 02 00:06:44 stroemsoe kernel: [] ? wake_threads_waitq+0x30/0x30 20 Feb 02 00:06:44 stroemsoe kernel: [] ? irq_thread_dtor+0xc0/0xc0 21 Feb 02 00:06:44 stroemsoe kernel: [] kthread+0xea/0x100 22 Feb 02 00:06:44 stroemsoe kernel: [] ? kthread_create_on_node+0x1c0/0x1c0 23 Feb 02 00:06:44 stroemsoe kernel: [] ret_from_fork+0x7c/0xb0 24 Feb 02 00:06:44 stroemsoe kernel: [] ? kthread_create_on_node+0x1c0/0x1c0 25 Feb 02 00:06:44 stroemsoe kernel: ---[ end trace 46f2548918f46443 ]--- I get about 4-5 of them per second as long as compton is running. Starting compton with `--backend xrender` doesn't cause any warnings. I'm using the SNA AccelMethod (the default), but switching to UXA or Glamor didn't seem to have any effect. I haven't set any i915 specific kernel flags. I don't see this on the current stable 3.18.5 kernel. The first time I saw this was on 3.19rc3 when I tried it due to another bug in the 3.18 series. I haven't gone through all the 3.19 release candidates, but the behaviour with 3.19rc6 seems identical to what I saw with 3.19rc3. Sakari Kapanen ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove nested work in gpu error handling
On Mon, Feb 02, 2015 at 09:17:14AM +, Chris Wilson wrote: > On Wed, Jan 28, 2015 at 05:03:14PM +0200, Mika Kuoppala wrote: > > @@ -2616,6 +2612,9 @@ void i915_handle_error(struct drm_device *dev, bool > > wedged, > > va_list args; > > char error_msg[80]; > > > > + if (WARN_ON(mutex_is_locked(&dev_priv->dev->struct_mutex))) > > + return; > > + > > Oops, sorry, I should have realised this was wrong earlier. The mutex > breaking occurs later in i915_handle_error. Oh well, already merged. Also, prts seems to complain that a bunch of hang stress-tests changed from fail to timeout because of this one here. Is this patch accidentally fix a bug and we just need to tune the tests, or is there some new deadlock now? prts results are really thin, per usual :( Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> To be used from the new addfb2 extension.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> include/uapi/drm/i915_drm.h | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 6eed16b..a7327fd 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -28,6 +28,7 @@
> #define _UAPI_I915_DRM_H_
>
> #include
> +#include
>
> /* Please note that modifications to all structs defined here are
> * subject to backwards-compatibility constraints.
> @@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
> __u64 value;
> };
>
> +/** @{
> + * Intel framebuffer modifiers
> + *
> + * Tiling modes supported by the display hardware
> + * to be passed in via the DRM addfb2 ioctl.
> + */
> +/** None */
> +#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL, 0x00L)
> +/** X tiling */
> +#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL,
> 0x01L)
One thing I wonder here is whether we should have a modifier for each
physical layout (tiling modes do change slightly between hw) or whether we
should just continue to assume that this is Intel-specific and add a
disclaimer that the precise layout depends upon the actual intel box
you're running on?
Leaning towards your approach, worst case we get to write some code to
de-alias layout modifiers with established cross-vendor layouts (if they
ever happen). Just want to make sure that we've thought about this. Adding
Rob&dri-devel for this.
-Daniel
> +/** @} */
> +
> #endif /* _UAPI_I915_DRM_H_ */
> --
> 2.2.2
>
> ___
> 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/6] drm/i915: Use framebuffer tiling mode for display purposes
On Fri, Jan 30, 2015 at 05:36:56PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> To prepare for framebuffer modifiers, move tiling definition from the
> object into the framebuffer. Move in a way that framebuffer tiling is
> now used for display while object tiling remains for fencing.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> drivers/gpu/drm/i915/intel_display.c | 46
> +---
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_pm.c | 7 +++---
> drivers/gpu/drm/i915/intel_sprite.c | 26 ++--
> 4 files changed, 46 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 4425e86..e22afbe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2211,7 +2211,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - switch (obj->tiling_mode) {
> + switch (to_intel_framebuffer(fb)->tiling_mode) {
> case I915_TILING_NONE:
Imo we should just look at fb->modifier[0] and flip over all the enums. A
bit more invasive, but we also don't need to change all the platform code
at once - set_tiling already guarantees that no one can modify the bo
tiling mode when there's an fb object using it. Which means we can change
the code over at leasure.
> if (INTEL_INFO(dev)->gen >= 9)
> alignment = 256 * 1024;
> @@ -2474,6 +2474,7 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
> u32 dspcntr;
> u32 reg = DSPCNTR(plane);
> int pixel_size;
> + unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
>
> if (!intel_crtc->primary_enabled) {
> I915_WRITE(reg, 0);
> @@ -2545,8 +2546,7 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
> BUG();
> }
>
> - if (INTEL_INFO(dev)->gen >= 4 &&
> - obj->tiling_mode != I915_TILING_NONE)
> + if (INTEL_INFO(dev)->gen >= 4 && tiling_mode != I915_TILING_NONE)
> dspcntr |= DISPPLANE_TILED;
>
> if (IS_G4X(dev))
> @@ -2556,7 +2556,8 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
>
> if (INTEL_INFO(dev)->gen >= 4) {
> intel_crtc->dspaddr_offset =
> - intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
> + intel_gen4_compute_page_offset(&x, &y,
> +tiling_mode,
> pixel_size,
> fb->pitches[0]);
> linear_offset -= intel_crtc->dspaddr_offset;
> @@ -2606,6 +2607,7 @@ static void ironlake_update_primary_plane(struct
> drm_crtc *crtc,
> u32 dspcntr;
> u32 reg = DSPCNTR(plane);
> int pixel_size;
> + unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
>
> if (!intel_crtc->primary_enabled) {
> I915_WRITE(reg, 0);
> @@ -2654,7 +2656,7 @@ static void ironlake_update_primary_plane(struct
> drm_crtc *crtc,
> BUG();
> }
>
> - if (obj->tiling_mode != I915_TILING_NONE)
> + if (tiling_mode != I915_TILING_NONE)
> dspcntr |= DISPPLANE_TILED;
>
> if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
> @@ -2662,7 +2664,8 @@ static void ironlake_update_primary_plane(struct
> drm_crtc *crtc,
>
> linear_offset = y * fb->pitches[0] + x * pixel_size;
> intel_crtc->dspaddr_offset =
> - intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
> + intel_gen4_compute_page_offset(&x, &y,
> +tiling_mode,
> pixel_size,
> fb->pitches[0]);
> linear_offset -= intel_crtc->dspaddr_offset;
> @@ -2750,7 +2753,7 @@ static void skylake_update_primary_plane(struct
> drm_crtc *crtc,
>* The stride is either expressed as a multiple of 64 bytes chunks for
>* linear buffers or in number of tiles for tiled buffers.
>*/
> - switch (obj->tiling_mode) {
> + switch (to_intel_framebuffer(fb)->tiling_mode) {
> case I915_TILING_NONE:
> stride = fb->pitches[0] >> 6;
> break;
> @@ -9291,7 +9294,7 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
> MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
> intel_ring_emit(ring, fb->pitches[0]);
> intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset |
> - obj->tiling_mode);
> + to_intel_framebuffer(fb)->tiling_mode);
>
> /* XXX Enabling the panel-fitter across page-flip is so far
>* untested on non-native modes, so ignore it for now.
> @@ -9324,7 +9327,8
Re: [Intel-gfx] [RFC 6/6] drm/i915: Announce support for framebuffer modifiers
On Fri, Jan 30, 2015 at 05:36:58PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > Let the DRM core know we can handle it. > > Signed-off-by: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/intel_display.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index ca69da0..1a8d433 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13202,6 +13202,8 @@ void intel_modeset_init(struct drm_device *dev) > dev->mode_config.preferred_depth = 24; > dev->mode_config.prefer_shadow = 1; > > + dev->mode_config.allow_fb_modifiers = 1; Bikeshed: s/1/true/ (and perhaps do that for prefer_shadow too in a trivial patch ...). -Daniel > + > dev->mode_config.funcs = &intel_mode_funcs; > > intel_init_quirks(dev); > -- > 2.2.2 > > ___ > 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 5/6] drm/i915: Allow fb modifier to set framebuffer tiling
On Fri, Jan 30, 2015 at 05:36:57PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Use the fb modifier if it was specified over object tiling mode.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> drivers/gpu/drm/i915/intel_display.c | 40
> +---
> 1 file changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e22afbe..ca69da0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12671,6 +12671,20 @@ static const struct drm_framebuffer_funcs
> intel_fb_funcs = {
> .create_handle = intel_user_framebuffer_create_handle,
> };
>
> +static unsigned int
> +intel_fb_modifier_to_tiling(u64 modifier)
> +{
> + switch (modifier) {
> + case I915_FORMAT_MOD_X_TILED:
> + return I915_TILING_X;
> + default:
> + case I915_FORMAT_MOD_NONE:
> + break;
> + }
> +
> + return I915_TILING_NONE;
> +}
> +
> static int intel_framebuffer_init(struct drm_device *dev,
> struct intel_framebuffer *intel_fb,
> struct drm_mode_fb_cmd2 *mode_cmd,
> @@ -12678,11 +12692,23 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> {
> int aligned_height;
> int pitch_limit;
> + unsigned int tiling_mode = obj->tiling_mode;
> int ret;
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (obj->tiling_mode == I915_TILING_Y) {
> + if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> + tiling_mode =
> + intel_fb_modifier_to_tiling(mode_cmd->modifier[0]);
> + if (tiling_mode != obj->tiling_mode &&
> + obj->tiling_mode != I915_TILING_NONE) {
> + DRM_ERROR("Tiling modifier mismatch %u vs obj %u!\n",
> + tiling_mode, obj->tiling_mode);
> + return -EINVAL;
> + }
> + }
Ah, here comes the magic. I think this might be simpler if we just use
->modifier (and fix it up if FB_MODIFIERS isn't set).
Btw another reason for this split is that this way we have a clear
separation between the tiling modes supported generally (as fb modifiers)
and the tiling modes supported by fences. It might therefore make sense to
rename obj->tiling_mode with a cocci patch to obj->fencing_mode or
->fence_tiling_mode). To make it really clear that it's just about the
global gtt fences and nothing more.
-Daniel
> +
> + if (tiling_mode == I915_TILING_Y) {
> DRM_DEBUG("hardware does not support tiling Y\n");
> return -EINVAL;
> }
> @@ -12696,12 +12722,12 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> if (INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev)) {
> pitch_limit = 32*1024;
> } else if (INTEL_INFO(dev)->gen >= 4) {
> - if (obj->tiling_mode)
> + if (tiling_mode)
> pitch_limit = 16*1024;
> else
> pitch_limit = 32*1024;
> } else if (INTEL_INFO(dev)->gen >= 3) {
> - if (obj->tiling_mode)
> + if (tiling_mode)
> pitch_limit = 8*1024;
> else
> pitch_limit = 16*1024;
> @@ -12711,12 +12737,12 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
>
> if (mode_cmd->pitches[0] > pitch_limit) {
> DRM_DEBUG("%s pitch (%d) must be at less than %d\n",
> - obj->tiling_mode ? "tiled" : "linear",
> + tiling_mode ? "tiled" : "linear",
> mode_cmd->pitches[0], pitch_limit);
> return -EINVAL;
> }
>
> - if (obj->tiling_mode != I915_TILING_NONE &&
> + if (tiling_mode != I915_TILING_NONE && obj->stride &&
> mode_cmd->pitches[0] != obj->stride) {
> DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n",
> mode_cmd->pitches[0], obj->stride);
> @@ -12771,7 +12797,7 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> return -EINVAL;
>
> aligned_height = intel_fb_align_height(dev, mode_cmd->height,
> -obj->tiling_mode);
> +tiling_mode);
> /* FIXME drm helper for size checks (especially planar formats)? */
> if (obj->base.size < aligned_height * mode_cmd->pitches[0])
> return -EINVAL;
> @@ -12779,7 +12805,7 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
> intel_fb->obj = obj;
> intel_fb->obj->framebuffer_references++;
> - intel_fb->tiling_mode = obj->tiling_mode;
> + intel_fb->tiling_mode = tiling_mode;
>
> ret = drm_framebuffer_i
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On Mon, Feb 02, 2015 at 10:41:24AM +0100, Daniel Vetter wrote:
> On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin
> >
> > To be used from the new addfb2 extension.
> >
> > Signed-off-by: Tvrtko Ursulin
> > ---
> > include/uapi/drm/i915_drm.h | 13 +
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index 6eed16b..a7327fd 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -28,6 +28,7 @@
> > #define _UAPI_I915_DRM_H_
> >
> > #include
> > +#include
> >
> > /* Please note that modifications to all structs defined here are
> > * subject to backwards-compatibility constraints.
> > @@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
> > __u64 value;
> > };
> >
> > +/** @{
> > + * Intel framebuffer modifiers
> > + *
> > + * Tiling modes supported by the display hardware
> > + * to be passed in via the DRM addfb2 ioctl.
> > + */
> > +/** None */
> > +#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL,
> > 0x00L)
> > +/** X tiling */
> > +#define I915_FORMAT_MOD_X_TILEDfourcc_mod_code(INTEL,
> > 0x01L)
>
> One thing I wonder here is whether we should have a modifier for each
> physical layout (tiling modes do change slightly between hw) or whether we
> should just continue to assume that this is Intel-specific and add a
> disclaimer that the precise layout depends upon the actual intel box
> you're running on?
>
> Leaning towards your approach, worst case we get to write some code to
> de-alias layout modifiers with established cross-vendor layouts (if they
> ever happen). Just want to make sure that we've thought about this. Adding
> Rob&dri-devel for this.
Something else to ponder: We also need layout modifiers for non-fb formats
in userspace so that clients and compositors can communicate about render
formats. Given that I think it'll make sense to enumerate all the other
tiling formats we have, too (i.e. Y-tiled and W-tiled).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove nested work in gpu error handling
On Mon, Feb 02, 2015 at 10:38:19AM +0100, Daniel Vetter wrote: > On Mon, Feb 02, 2015 at 09:17:14AM +, Chris Wilson wrote: > > On Wed, Jan 28, 2015 at 05:03:14PM +0200, Mika Kuoppala wrote: > > > @@ -2616,6 +2612,9 @@ void i915_handle_error(struct drm_device *dev, bool > > > wedged, > > > va_list args; > > > char error_msg[80]; > > > > > > + if (WARN_ON(mutex_is_locked(&dev_priv->dev->struct_mutex))) > > > + return; > > > + > > > > Oops, sorry, I should have realised this was wrong earlier. The mutex > > breaking occurs later in i915_handle_error. > > Oh well, already merged. Also, prts seems to complain that a bunch of > hang stress-tests changed from fail to timeout because of this one here. > Is this patch accidentally fix a bug and we just need to tune the tests, > or is there some new deadlock now? prts results are really thin, per usual > :( Yes. It will also prevent the gpu reset which those tests depend upon. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On 02/02/2015 09:58 AM, Daniel Vetter wrote:
On Mon, Feb 02, 2015 at 10:41:24AM +0100, Daniel Vetter wrote:
On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
To be used from the new addfb2 extension.
Signed-off-by: Tvrtko Ursulin
---
include/uapi/drm/i915_drm.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6eed16b..a7327fd 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -28,6 +28,7 @@
#define _UAPI_I915_DRM_H_
#include
+#include
/* Please note that modifications to all structs defined here are
* subject to backwards-compatibility constraints.
@@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
__u64 value;
};
+/** @{
+ * Intel framebuffer modifiers
+ *
+ * Tiling modes supported by the display hardware
+ * to be passed in via the DRM addfb2 ioctl.
+ */
+/** None */
+#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL, 0x00L)
+/** X tiling */
+#define I915_FORMAT_MOD_X_TILEDfourcc_mod_code(INTEL,
0x01L)
One thing I wonder here is whether we should have a modifier for each
physical layout (tiling modes do change slightly between hw) or whether we
should just continue to assume that this is Intel-specific and add a
disclaimer that the precise layout depends upon the actual intel box
you're running on?
Leaning towards your approach, worst case we get to write some code to
de-alias layout modifiers with established cross-vendor layouts (if they
ever happen). Just want to make sure that we've thought about this. Adding
Rob&dri-devel for this.
Something else to ponder: We also need layout modifiers for non-fb formats
in userspace so that clients and compositors can communicate about render
formats. Given that I think it'll make sense to enumerate all the other
tiling formats we have, too (i.e. Y-tiled and W-tiled).
If we need fb modifiers for non-fb formats, although that sounds a bit
funky to me, we can always add them in separate patches, no?
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/6] drm/i915: Use framebuffer tiling mode for display purposes
On 02/02/2015 09:49 AM, Daniel Vetter wrote:
On Fri, Jan 30, 2015 at 05:36:56PM +, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
To prepare for framebuffer modifiers, move tiling definition from the
object into the framebuffer. Move in a way that framebuffer tiling is
now used for display while object tiling remains for fencing.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/intel_display.c | 46 +---
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 7 +++---
drivers/gpu/drm/i915/intel_sprite.c | 26 ++--
4 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4425e86..e22afbe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2211,7 +2211,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- switch (obj->tiling_mode) {
+ switch (to_intel_framebuffer(fb)->tiling_mode) {
case I915_TILING_NONE:
Imo we should just look at fb->modifier[0] and flip over all the enums. A
bit more invasive, but we also don't need to change all the platform code
at once - set_tiling already guarantees that no one can modify the bo
tiling mode when there's an fb object using it. Which means we can change
the code over at leasure.
What do you mean by "flip over"?
To make places which need to get fb tiling format use fb->modifier[0]
directly rather than have them mapped to tiling enums at fb init?
@@ -9764,6 +9768,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->event = event;
work->crtc = crtc;
work->old_fb_obj = intel_fb_obj(old_fb);
+ work->old_tiling_mode = to_intel_framebuffer(old_fb)->tiling_mode;
Hm, that's actually an interesting bugfix - currently userspace could be
sneaky and destroy the old fb immediately after the flip completes and the
change the tiling of the underlying object before the unpin work had a
chance to run (needs some fudgin with rt prios to starve workers to make
this work though).
Imo the right fix is to hold a reference onto the fb and not the
underlying gem object. With that tiling is guaranteed not to change.
Ok I'll pull it out in a separate patch.
INIT_WORK(&work->work, intel_unpin_work_fn);
ret = drm_crtc_vblank_get(crtc);
@@ -9814,7 +9819,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
if (IS_VALLEYVIEW(dev)) {
ring = &dev_priv->ring[BCS];
- if (obj->tiling_mode != work->old_fb_obj->tiling_mode)
+ if (to_intel_framebuffer(fb)->tiling_mode !=
+ work->old_tiling_mode)
/* vlv: DISPLAY_FLIP fails to change tiling */
ring = NULL;
} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
@@ -12190,7 +12196,8 @@ intel_check_cursor_plane(struct drm_plane *plane,
/* we only need to pin inside GTT if cursor is non-phy */
mutex_lock(&dev->struct_mutex);
- if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) {
+ if (!INTEL_INFO(dev)->cursor_needs_physical &&
+ to_intel_framebuffer(fb)->tiling_mode) {
DRM_DEBUG_KMS("cursor cannot be tiled\n");
ret = -EINVAL;
}
@@ -12772,6 +12779,7 @@ static int intel_framebuffer_init(struct drm_device
*dev,
drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
intel_fb->obj = obj;
intel_fb->obj->framebuffer_references++;
+ intel_fb->tiling_mode = obj->tiling_mode;
One side-effect of using fb->modifier[0] is that if the modifier flag is
_not_ set, we need to reconstruct this field from obj->tiling_mode here.
Otoh if it is set this code here should check that fb->modifier and
obj->tiling_mode are consistent.
Perhaps best to split this change out as a prep patch, like you've done
with the code for the initial framebuffer.
If I understood correctly what you meant in the first quote then yes.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 5/6] drm/i915: Allow fb modifier to set framebuffer tiling
On 02/02/2015 09:54 AM, Daniel Vetter wrote:
On Fri, Jan 30, 2015 at 05:36:57PM +, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Use the fb modifier if it was specified over object tiling mode.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/intel_display.c | 40 +---
1 file changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index e22afbe..ca69da0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12671,6 +12671,20 @@ static const struct drm_framebuffer_funcs
intel_fb_funcs = {
.create_handle = intel_user_framebuffer_create_handle,
};
+static unsigned int
+intel_fb_modifier_to_tiling(u64 modifier)
+{
+ switch (modifier) {
+ case I915_FORMAT_MOD_X_TILED:
+ return I915_TILING_X;
+ default:
+ case I915_FORMAT_MOD_NONE:
+ break;
+ }
+
+ return I915_TILING_NONE;
+}
+
static int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *intel_fb,
struct drm_mode_fb_cmd2 *mode_cmd,
@@ -12678,11 +12692,23 @@ static int intel_framebuffer_init(struct drm_device
*dev,
{
int aligned_height;
int pitch_limit;
+ unsigned int tiling_mode = obj->tiling_mode;
int ret;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (obj->tiling_mode == I915_TILING_Y) {
+ if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
+ tiling_mode =
+ intel_fb_modifier_to_tiling(mode_cmd->modifier[0]);
+ if (tiling_mode != obj->tiling_mode &&
+ obj->tiling_mode != I915_TILING_NONE) {
+ DRM_ERROR("Tiling modifier mismatch %u vs obj %u!\n",
+ tiling_mode, obj->tiling_mode);
+ return -EINVAL;
+ }
+ }
Ah, here comes the magic. I think this might be simpler if we just use
->modifier (and fix it up if FB_MODIFIERS isn't set).
Btw another reason for this split is that this way we have a clear
separation between the tiling modes supported generally (as fb modifiers)
and the tiling modes supported by fences. It might therefore make sense to
rename obj->tiling_mode with a cocci patch to obj->fencing_mode or
->fence_tiling_mode). To make it really clear that it's just about the
global gtt fences and nothing more.
I don't really like using ->modifier directly in tiling patch since it
is an bag of unrelated stuff, not only a superset. Unrelated especially,
but not only, from the point of view of call sites / users.
Therefore I see some design elegance in extracting the tiling, or any
other logical group of modifiers before hand.
At the very least would call something like
intel_fb_modifier_to_tiling(), but, it is very ugly to have a dynamic
cost at every call site. Which is another reason why I preferred to
extract the data before hand.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/skl: Split the SKL PCI ids by GT
On Fri, Jan 30, 2015 at 09:30:07AM +0200, Jani Nikula wrote: > On a related note, I'm contemplating sending a patch to obliterate the > _INTEL_BDW_M and _INTEL_BDW_D macros from i915_pciids.h because it hides > the IDs from a simple grep. Yes, please! -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/bdw: Implement non-coherent ctx w/a
On Thu, Jan 08, 2015 at 07:59:10PM -0800, Ben Widawsky wrote:
> Implements a required workaround whose implications aren't entirely clear to
> me
> from the description. In particular I do not know if this effects legacy
> contexts, execlists, or both.
>
> I couldn't find a real workaround name, so I made up:
> WaHdcCtxNonCoherent
I don't think we want to make up w/a names. Might cause someone to
conclude that the w/a is no longer needed if they can't find the
name in the w/a database or bspec. So maybe just add a small quote from
bspec, or leave it without explanation forcing people to check bspec
if they want to find out why it's there.
I suppose one option would be to add a private namespace for our made
up w/a names. But I don't really see a point in making up w/a names
if we don't have a some documentation telling people what those names
actually mean.
So with the made up w/a name removed:
Reviewed-by: Ville Syrjälä
>
> Signed-off-by: Ben Widawsky
> ---
> drivers/gpu/drm/i915/i915_reg.h | 5 +++--
> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0f32fd1a..dabac96 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5219,9 +5219,10 @@ enum punit_power_well {
>
> /* GEN8 chicken */
> #define HDC_CHICKEN0 0x7300
> -#define HDC_FORCE_NON_COHERENT (1<<4)
> -#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11)
> #define HDC_FENCE_DEST_SLM_DISABLE (1<<14)
> +#define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11)
> +#define HDC_FORCE_CTX_NON_COHERENT (1<<5)
> +#define HDC_FORCE_NON_COHERENT (1<<4)
>
> /* WaCatErrorRejectionIssue */
> #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG 0x9030
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 12a36f0..62318a4 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -790,8 +790,10 @@ static int bdw_init_workarounds(struct intel_engine_cs
> *ring)
>*/
> /* WaForceEnableNonCoherent:bdw */
> /* WaHdcDisableFetchWhenMasked:bdw */
> + /* WaHdcCtxNonCoherent:bdw */
> /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production) */
> WA_SET_BIT_MASKED(HDC_CHICKEN0,
> + HDC_FORCE_CTX_NON_COHERENT |
> HDC_FORCE_NON_COHERENT |
> HDC_DONOT_FETCH_MEM_WHEN_MASKED |
> (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0));
> --
> 2.2.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/bdw: Implement non-coherent ctx w/a
On Mon, Feb 02, 2015 at 02:33:48PM +0200, Ville Syrjälä wrote: > On Thu, Jan 08, 2015 at 07:59:10PM -0800, Ben Widawsky wrote: > > Implements a required workaround whose implications aren't entirely clear > > to me > > from the description. In particular I do not know if this effects legacy > > contexts, execlists, or both. > > > > I couldn't find a real workaround name, so I made up: > > WaHdcCtxNonCoherent > > I don't think we want to make up w/a names. Might cause someone to > conclude that the w/a is no longer needed if they can't find the > name in the w/a database or bspec. So maybe just add a small quote from > bspec, or leave it without explanation forcing people to check bspec > if they want to find out why it's there. > > I suppose one option would be to add a private namespace for our made > up w/a names. But I don't really see a point in making up w/a names > if we don't have a some documentation telling people what those names > actually mean. > > So with the made up w/a name removed: > Reviewed-by: Ville Syrjälä If you want to believe my version, it's called WaForceContextSaveRestoreNonCoherent http://lists.freedesktop.org/archives/intel-gfx/2015-January/059086.html -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Implement WaForceContextSaveRestoreNonCoherent
On Tue, Jan 27, 2015 at 06:13:13PM +, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau
OK, so this has the right w/a name whereas Ben's earlier patch didn't.
So we should go with this version instead.
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ae8ea42..47bc4e2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5253,6 +5253,7 @@ enum punit_power_well {
>
> /* GEN8 chicken */
> #define HDC_CHICKEN0 0x7300
> +#define HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT (1<<5)
> #define HDC_FORCE_NON_COHERENT (1<<4)
> #define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11)
> #define HDC_FENCE_DEST_SLM_DISABLE (1<<14)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 36dad33..d393026 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -790,9 +790,11 @@ static int bdw_init_workarounds(struct intel_engine_cs
> *ring)
>*/
> /* WaForceEnableNonCoherent:bdw */
> /* WaHdcDisableFetchWhenMasked:bdw */
> + /* WaForceContextSaveRestoreNonCoherent:bdw */
> /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production) */
> WA_SET_BIT_MASKED(HDC_CHICKEN0,
> HDC_FORCE_NON_COHERENT |
> + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT |
> HDC_DONOT_FETCH_MEM_WHEN_MASKED |
> (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0));
>
> --
> 1.8.3.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] memcg, shmem: fix shmem migration to use lrucare. (was: Re: memcontrol.c BUG)
On Thu 29-01-15 18:04:15, Hugh Dickins wrote:
> On Wed, 28 Jan 2015, Michal Hocko wrote:
> > On Wed 28-01-15 08:48:52, Chris Wilson wrote:
> > > On Wed, Jan 28, 2015 at 08:13:06AM +1000, Dave Airlie wrote:
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1165369
> > > >
> > > > ov 18 09:23:22 elissa.gathman.org kernel: page:f5e36a40 count:2
> > > > mapcount:0 mapping: (null) index:0x0
> > > > Nov 18 09:23:22 elissa.gathman.org kernel: page flags:
> > > > 0x80090029(locked|uptodate|lru|swapcache|swapbacked)
> > > > Nov 18 09:23:22 elissa.gathman.org kernel: page dumped because:
> > > > VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage))
> > > > Nov 18 09:23:23 elissa.gathman.org kernel: [ cut here
> > > > ]
> > > > Nov 18 09:23:23 elissa.gathman.org kernel: kernel BUG at
> > > > mm/memcontrol.c:6733!
> >
> > I guess this matches the following bugon in your kernel:
> > VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage), oldpage);
> >
> > so the oldpage is on the LRU list already. I am completely unfamiliar
> > with 965GM but is the page perhaps shared with somebody with a different
> > gfp mask requirement (e.g. userspace accessing the memory via mmap)? So
> > the other (racing) caller didn't need to move the page and put it on
> > LRU.
>
> It would be surprising (but not impossible) for oldpage not to be on
> the LRU already: it's a swapin readahead page that has every right to
> be on LRU,
True, thanks for pointing this out.
> but turns out to have been allocated from an unsuitable zone,
> once we discover that it's needed in one of these odd hardware-limited
> mappings. (Whereas newpage is newly allocated and not yet on LRU.)
>
> >
> > If yes we need to tell shmem_replace_page to do the lrucare handling.
>
> Absolutely, thanks Michal. It would also be good to change the comment
> on mem_cgroup_migrate() in mm/memcontrol.c, from "@lrucare: both pages..."
> to "@lrucare: either or both pages..." - though I certainly won't pretend
> that the corrected wording would have prevented this bug creeping in!
Yes, I have updated the wording.
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index 339e06639956..e3cdc1a16c0f 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -1013,7 +1013,7 @@ static int shmem_replace_page(struct page **pagep,
> > gfp_t gfp,
> > */
> > oldpage = newpage;
> > } else {
> > - mem_cgroup_migrate(oldpage, newpage, false);
> > + mem_cgroup_migrate(oldpage, newpage, true);
> > lru_cache_add_anon(newpage);
> > *pagep = newpage;
> > }
>
> Acked-by: Hugh Dickins
Thanks! The full patch is below. I wasn't sure who was the one to report
the issue so I hope the credits are right. I have marked the patch for
stable because some people are running with VM debugging enabled. AFAICS
the issue is not so harmful without debugging on because the stale
oldpage would be removed from the LRU list eventually.
---
From 508815bfdaae75e3286ab2dd714a07201665709c Mon Sep 17 00:00:00 2001
From: Michal Hocko
Date: Mon, 2 Feb 2015 15:22:19 +0100
Subject: [PATCH] memcg, shmem: fix shmem migration to use lrucare.
It has been reported that 965GM might trigger
VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage), oldpage)
in mem_cgroup_migrate when shmem wants to replace a swap cache page
because of shmem_should_replace_page (the page is allocated from an
inappropriate zone). shmem_replace_page expects that the oldpage is not
on LRU list and calls mem_cgroup_migrate without lrucare. This is obviously
incorrect because swapcache pages might be on the LRU list (e.g. swapin
readahead page).
Fix this by enabling lrucare for the migration in shmem_replace_page.
Also clarify that lrucare should be used even if one of the pages might
be on LRU list.
The BUG_ON will trigger only when CONFIG_DEBUG_VM is enabled but even
without that the migration code might leave the old page on an
inappropriate memcg' LRU which is not that critical because the page
would get removed with its last reference but it is still confusing.
Fixes: 0a31bc97c80c (mm: memcontrol: rewrite uncharge API)
Cc: sta...@vger.kernel.org # 3.17+
Reported-by: Chris Wilson
Reported-by: Dave Airlie
Acked-by: Hugh Dickins
Signed-off-by: Michal Hocko
---
mm/memcontrol.c | 2 +-
mm/shmem.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7ce5aa24bc19..c5ac0e209868 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5743,7 +5743,7 @@ void mem_cgroup_uncharge_list(struct list_head *page_list)
* mem_cgroup_migrate - migrate a charge to another page
* @oldpage: currently charged page
* @newpage: page to transfer the charge to
- * @lrucare: both pages might be on the LRU already
+ * @lrucare: either or both pages might be on the LRU already
*
* Migrate the charge from @oldpage to @newpage.
*
diff --git a/mm/shmem.c b/mm/shmem.c
index 339e06639956..e3cdc1a16c0f 100644
--- a/mm/
[Intel-gfx] Significance of Golden context
Hi, Could someone explain the significance of Null context/Golden state? I understand we are initializing 3D state in this batch and we send this at the beginning to start the HW with a known state but what are implications of not doing this? what kind of issues we can expect if we don't do this? How is this golden state determined? As a test I disabled this for Gen8 and I can boot Android without any issues. regards Arun ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Enable eDRAM for gen9 as well
On Fri, Jan 30, 2015 at 05:23:06PM +0100, Daniel Vetter wrote: > On Thu, Jan 29, 2015 at 12:42:35PM +, Damien Lespiau wrote: > > Suggested-by: Daniel Vetter > > Signed-off-by: Damien Lespiau > > Hm, I've thought the magic bit moved ... or have you found it in configdb > again? The eDRAM present bit is still bit 0. However the code has changed and the patch needs rebasing already. Also I realized we don't actually select bit 0 and compare with the full register value. Patches to follow... -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/documentation: Add intel_uncore.c to drm.tmpl
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5677 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 353/353 353/353 ILK 355/355 355/355 SNB 400/422 400/422 IVB +2 485/487 487/487 BYT 296/296 296/296 HSW +1-2 507/508 506/508 BDW -2 401/402 399/402 -Detailed- Platform Testdrm-intel-nightly Series Applied IVB igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(7, M34M21)PASS(10, M4M34) PASS(1, M21) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(6, M34M4)PASS(18, M34M4M21) PASS(1, M21) HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(1, M40)PASS(22, M40M20) PASS(1, M20) HSW igt_gem_storedw_loop_blt DMESG_WARN(1, M20)PASS(3, M40M20) DMESG_WARN(1, M20) *HSW igt_gem_partial_pwrite_pread_write PASS(2, M40M20) DMESG_WARN(1, M20) BDW igt_gem_pwrite_pread_display-pwrite-blt-gtt_mmap-performance DMESG_WARN(5, M28)PASS(3, M30M28) DMESG_WARN(1, M28) BDW igt_gem_pwrite_pread_uncached-pwrite-blt-gtt_mmap-performance DMESG_WARN(1, M28)PASS(7, M30M28) DMESG_WARN(1, M28) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Track old framebuffer instead of object
From: Tvrtko Ursulin
Daniel Vetter spotted a bug while reviewing some of my refactoring in this
are of the code. I'll quote:
"""
> @@ -9764,6 +9768,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> work->event = event;
> work->crtc = crtc;
> work->old_fb_obj = intel_fb_obj(old_fb);
> + work->old_tiling_mode = to_intel_framebuffer(old_fb)->tiling_mode;
Hm, that's actually an interesting bugfix - currently userspace could be
sneaky and destroy the old fb immediately after the flip completes and the
change the tiling of the underlying object before the unpin work had a
chance to run (needs some fudgin with rt prios to starve workers to make
this work though).
Imo the right fix is to hold a reference onto the fb and not the
underlying gem object. With that tiling is guaranteed not to change.
"""
This patch tries to implement the above proposed change.
Signed-off-by: Tvrtko Ursulin
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 14 +++---
drivers/gpu/drm/i915/intel_drv.h | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1a689b3..24904cc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9111,9 +9111,9 @@ static void intel_unpin_work_fn(struct work_struct
*__work)
enum pipe pipe = to_intel_crtc(work->crtc)->pipe;
mutex_lock(&dev->struct_mutex);
- intel_unpin_fb_obj(work->old_fb_obj);
+ intel_unpin_fb_obj(intel_fb_obj(work->old_fb));
drm_gem_object_unreference(&work->pending_flip_obj->base);
- drm_gem_object_unreference(&work->old_fb_obj->base);
+ drm_framebuffer_unreference(work->old_fb);
intel_fbc_update(dev);
@@ -9816,7 +9816,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->event = event;
work->crtc = crtc;
- work->old_fb_obj = intel_fb_obj(old_fb);
+ work->old_fb = old_fb;
INIT_WORK(&work->work, intel_unpin_work_fn);
ret = drm_crtc_vblank_get(crtc);
@@ -9852,7 +9852,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
goto cleanup;
/* Reference the objects for the scheduled work. */
- drm_gem_object_reference(&work->old_fb_obj->base);
+ drm_framebuffer_reference(work->old_fb);
drm_gem_object_reference(&obj->base);
crtc->primary->fb = fb;
@@ -9867,7 +9867,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
if (IS_VALLEYVIEW(dev)) {
ring = &dev_priv->ring[BCS];
- if (obj->tiling_mode != work->old_fb_obj->tiling_mode)
+ if (obj->tiling_mode != intel_fb_obj(work->old_fb)->tiling_mode)
/* vlv: DISPLAY_FLIP fails to change tiling */
ring = NULL;
} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
@@ -9908,7 +9908,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->flip_queued_vblank = drm_vblank_count(dev, intel_crtc->pipe);
work->enable_stall_check = true;
- i915_gem_track_fb(work->old_fb_obj, obj,
+ i915_gem_track_fb(intel_fb_obj(work->old_fb), obj,
INTEL_FRONTBUFFER_PRIMARY(pipe));
intel_fbc_disable(dev);
@@ -9924,7 +9924,7 @@ cleanup_unpin:
cleanup_pending:
atomic_dec(&intel_crtc->unpin_work_count);
crtc->primary->fb = old_fb;
- drm_gem_object_unreference(&work->old_fb_obj->base);
+ drm_framebuffer_unreference(work->old_fb);
drm_gem_object_unreference(&obj->base);
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index de513ec..a8f895f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -712,7 +712,7 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane)
struct intel_unpin_work {
struct work_struct work;
struct drm_crtc *crtc;
- struct drm_i915_gem_object *old_fb_obj;
+ struct drm_framebuffer *old_fb;
struct drm_i915_gem_object *pending_flip_obj;
struct drm_pending_vblank_event *event;
atomic_t pending;
--
2.2.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On Mon, Feb 02, 2015 at 10:23:57AM +, Tvrtko Ursulin wrote:
>
> On 02/02/2015 09:58 AM, Daniel Vetter wrote:
> >On Mon, Feb 02, 2015 at 10:41:24AM +0100, Daniel Vetter wrote:
> >>On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
> >>>From: Tvrtko Ursulin
> >>>
> >>>To be used from the new addfb2 extension.
> >>>
> >>>Signed-off-by: Tvrtko Ursulin
> >>>---
> >>> include/uapi/drm/i915_drm.h | 13 +
> >>> 1 file changed, 13 insertions(+)
> >>>
> >>>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> >>>index 6eed16b..a7327fd 100644
> >>>--- a/include/uapi/drm/i915_drm.h
> >>>+++ b/include/uapi/drm/i915_drm.h
> >>>@@ -28,6 +28,7 @@
> >>> #define _UAPI_I915_DRM_H_
> >>>
> >>> #include
> >>>+#include
> >>>
> >>> /* Please note that modifications to all structs defined here are
> >>> * subject to backwards-compatibility constraints.
> >>>@@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
> >>> __u64 value;
> >>> };
> >>>
> >>>+/** @{
> >>>+ * Intel framebuffer modifiers
> >>>+ *
> >>>+ * Tiling modes supported by the display hardware
> >>>+ * to be passed in via the DRM addfb2 ioctl.
> >>>+ */
> >>>+/** None */
> >>>+#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL,
> >>>0x00L)
> >>>+/** X tiling */
> >>>+#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL,
> >>>0x01L)
> >>
> >>One thing I wonder here is whether we should have a modifier for each
> >>physical layout (tiling modes do change slightly between hw) or whether we
> >>should just continue to assume that this is Intel-specific and add a
> >>disclaimer that the precise layout depends upon the actual intel box
> >>you're running on?
> >>
> >>Leaning towards your approach, worst case we get to write some code to
> >>de-alias layout modifiers with established cross-vendor layouts (if they
> >>ever happen). Just want to make sure that we've thought about this. Adding
> >>Rob&dri-devel for this.
> >
> >Something else to ponder: We also need layout modifiers for non-fb formats
> >in userspace so that clients and compositors can communicate about render
> >formats. Given that I think it'll make sense to enumerate all the other
> >tiling formats we have, too (i.e. Y-tiled and W-tiled).
>
> If we need fb modifiers for non-fb formats, although that sounds a bit funky
> to me, we can always add them in separate patches, no?
Yes and no - I think the aliasing with the I915_TILING_FOO defines would
be nice, and if you reserve another number for the fancy new tiling you're
working on and so block Y-tiled that would be unfortunate ...
Otoh meh, we need to remap anyway sooner or later. Like I've said, just
something to consider.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On Mon, Feb 02, 2015 at 10:23:57AM +, Tvrtko Ursulin wrote:
>
> On 02/02/2015 09:58 AM, Daniel Vetter wrote:
> >On Mon, Feb 02, 2015 at 10:41:24AM +0100, Daniel Vetter wrote:
> >>On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
> >>>From: Tvrtko Ursulin
> >>>
> >>>To be used from the new addfb2 extension.
> >>>
> >>>Signed-off-by: Tvrtko Ursulin
> >>>---
> >>> include/uapi/drm/i915_drm.h | 13 +
> >>> 1 file changed, 13 insertions(+)
> >>>
> >>>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> >>>index 6eed16b..a7327fd 100644
> >>>--- a/include/uapi/drm/i915_drm.h
> >>>+++ b/include/uapi/drm/i915_drm.h
> >>>@@ -28,6 +28,7 @@
> >>> #define _UAPI_I915_DRM_H_
> >>>
> >>> #include
> >>>+#include
> >>>
> >>> /* Please note that modifications to all structs defined here are
> >>> * subject to backwards-compatibility constraints.
> >>>@@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
> >>> __u64 value;
> >>> };
> >>>
> >>>+/** @{
> >>>+ * Intel framebuffer modifiers
> >>>+ *
> >>>+ * Tiling modes supported by the display hardware
> >>>+ * to be passed in via the DRM addfb2 ioctl.
> >>>+ */
> >>>+/** None */
> >>>+#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL,
> >>>0x00L)
> >>>+/** X tiling */
> >>>+#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL,
> >>>0x01L)
> >>
> >>One thing I wonder here is whether we should have a modifier for each
> >>physical layout (tiling modes do change slightly between hw) or whether we
> >>should just continue to assume that this is Intel-specific and add a
> >>disclaimer that the precise layout depends upon the actual intel box
> >>you're running on?
> >>
> >>Leaning towards your approach, worst case we get to write some code to
> >>de-alias layout modifiers with established cross-vendor layouts (if they
> >>ever happen). Just want to make sure that we've thought about this. Adding
> >>Rob&dri-devel for this.
> >
> >Something else to ponder: We also need layout modifiers for non-fb formats
> >in userspace so that clients and compositors can communicate about render
> >formats. Given that I think it'll make sense to enumerate all the other
> >tiling formats we have, too (i.e. Y-tiled and W-tiled).
>
> If we need fb modifiers for non-fb formats, although that sounds a bit funky
> to me, we can always add them in separate patches, no?
Oh and the explanation of why this makes sense: Userspace needs to agree
on some modifier numbers assignment too for its purposes of sharing
buffers between clients and compositor. And there's a lot of overlap with
buffers that can actually be scanned out (for the obvious reason called
fullscreen apps), so it makes sense to reuse those numbers instead of
everyone creating their own spec.
But then we need to make sure that non-fb modifiers of interest as used in
userspace aren't eventually used by the kernel for something else. Hence
they need to go into the kernel headers, just to reserve the numbers.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On Mon, Feb 2, 2015 at 4:41 AM, Daniel Vetter wrote:
> On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin
>>
>> To be used from the new addfb2 extension.
>>
>> Signed-off-by: Tvrtko Ursulin
>> ---
>> include/uapi/drm/i915_drm.h | 13 +
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index 6eed16b..a7327fd 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -28,6 +28,7 @@
>> #define _UAPI_I915_DRM_H_
>>
>> #include
>> +#include
>>
>> /* Please note that modifications to all structs defined here are
>> * subject to backwards-compatibility constraints.
>> @@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
>> __u64 value;
>> };
>>
>> +/** @{
>> + * Intel framebuffer modifiers
>> + *
>> + * Tiling modes supported by the display hardware
>> + * to be passed in via the DRM addfb2 ioctl.
>> + */
>> +/** None */
>> +#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL, 0x00L)
>> +/** X tiling */
>> +#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL,
>> 0x01L)
>
> One thing I wonder here is whether we should have a modifier for each
> physical layout (tiling modes do change slightly between hw) or whether we
> should just continue to assume that this is Intel-specific and add a
> disclaimer that the precise layout depends upon the actual intel box
> you're running on?
I'd kind of lean towards different modifiers per physical layout..
that seems more useful for cases where nvidia/amd support some of the
formats for buffer sharing..
BR,
-R
> Leaning towards your approach, worst case we get to write some code to
> de-alias layout modifiers with established cross-vendor layouts (if they
> ever happen). Just want to make sure that we've thought about this. Adding
> Rob&dri-devel for this.
> -Daniel
>
>> +/** @} */
>> +
>> #endif /* _UAPI_I915_DRM_H_ */
>> --
>> 2.2.2
>>
>> ___
>> 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On Mon, Feb 2, 2015 at 10:58 AM, Daniel Vetter wrote:
> On Mon, Feb 02, 2015 at 10:23:57AM +, Tvrtko Ursulin wrote:
>>
>> On 02/02/2015 09:58 AM, Daniel Vetter wrote:
>> >On Mon, Feb 02, 2015 at 10:41:24AM +0100, Daniel Vetter wrote:
>> >>On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
>> >>>From: Tvrtko Ursulin
>> >>>
>> >>>To be used from the new addfb2 extension.
>> >>>
>> >>>Signed-off-by: Tvrtko Ursulin
>> >>>---
>> >>> include/uapi/drm/i915_drm.h | 13 +
>> >>> 1 file changed, 13 insertions(+)
>> >>>
>> >>>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> >>>index 6eed16b..a7327fd 100644
>> >>>--- a/include/uapi/drm/i915_drm.h
>> >>>+++ b/include/uapi/drm/i915_drm.h
>> >>>@@ -28,6 +28,7 @@
>> >>> #define _UAPI_I915_DRM_H_
>> >>>
>> >>> #include
>> >>>+#include
>> >>>
>> >>> /* Please note that modifications to all structs defined here are
>> >>> * subject to backwards-compatibility constraints.
>> >>>@@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
>> >>> __u64 value;
>> >>> };
>> >>>
>> >>>+/** @{
>> >>>+ * Intel framebuffer modifiers
>> >>>+ *
>> >>>+ * Tiling modes supported by the display hardware
>> >>>+ * to be passed in via the DRM addfb2 ioctl.
>> >>>+ */
>> >>>+/** None */
>> >>>+#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL,
>> >>>0x00L)
>> >>>+/** X tiling */
>> >>>+#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL,
>> >>>0x01L)
>> >>
>> >>One thing I wonder here is whether we should have a modifier for each
>> >>physical layout (tiling modes do change slightly between hw) or whether we
>> >>should just continue to assume that this is Intel-specific and add a
>> >>disclaimer that the precise layout depends upon the actual intel box
>> >>you're running on?
>> >>
>> >>Leaning towards your approach, worst case we get to write some code to
>> >>de-alias layout modifiers with established cross-vendor layouts (if they
>> >>ever happen). Just want to make sure that we've thought about this. Adding
>> >>Rob&dri-devel for this.
>> >
>> >Something else to ponder: We also need layout modifiers for non-fb formats
>> >in userspace so that clients and compositors can communicate about render
>> >formats. Given that I think it'll make sense to enumerate all the other
>> >tiling formats we have, too (i.e. Y-tiled and W-tiled).
>>
>> If we need fb modifiers for non-fb formats, although that sounds a bit funky
>> to me, we can always add them in separate patches, no?
>
> Oh and the explanation of why this makes sense: Userspace needs to agree
> on some modifier numbers assignment too for its purposes of sharing
> buffers between clients and compositor. And there's a lot of overlap with
> buffers that can actually be scanned out (for the obvious reason called
> fullscreen apps), so it makes sense to reuse those numbers instead of
> everyone creating their own spec.
>
> But then we need to make sure that non-fb modifiers of interest as used in
> userspace aren't eventually used by the kernel for something else. Hence
> they need to go into the kernel headers, just to reserve the numbers.
right.. the next logical step is to extend the egl dmabuf extension
to take modifiers in the same way as addfb2 does. So it makes sense
to reserve/enumerate any sharable modifier, even if it is not ever
used for scanout. As w/ fourcc's, it will be nice to keep the egl
extension to keep the same formats and modifiers.
BR,
-R
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Multiple declarations for intel_fbc_enabled
A FreeBSD developer discovered that intel_fbc_enabled has a declaration in two headers: sys/dev/drm2/i915/i915_drv.h:extern bool intel_fbc_enabled(struct drm_device *dev); sys/dev/drm2/i915/intel_drv.h:extern bool intel_fbc_enabled(struct drm_device *dev); We have a slightly older version of the i915 driver on FreeBSD, but I see that this is still the case in Linux (although the "extern" has been removed from one of them). Commit 85208be added the one in intel_drv.h and didn't remove the existing one. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On 02/02/2015 04:32 PM, Rob Clark wrote:
On Mon, Feb 2, 2015 at 4:41 AM, Daniel Vetter wrote:
On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
To be used from the new addfb2 extension.
Signed-off-by: Tvrtko Ursulin
---
include/uapi/drm/i915_drm.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6eed16b..a7327fd 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -28,6 +28,7 @@
#define _UAPI_I915_DRM_H_
#include
+#include
/* Please note that modifications to all structs defined here are
* subject to backwards-compatibility constraints.
@@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
__u64 value;
};
+/** @{
+ * Intel framebuffer modifiers
+ *
+ * Tiling modes supported by the display hardware
+ * to be passed in via the DRM addfb2 ioctl.
+ */
+/** None */
+#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL, 0x00L)
+/** X tiling */
+#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL, 0x01L)
One thing I wonder here is whether we should have a modifier for each
physical layout (tiling modes do change slightly between hw) or whether we
should just continue to assume that this is Intel-specific and add a
disclaimer that the precise layout depends upon the actual intel box
you're running on?
I'd kind of lean towards different modifiers per physical layout..
that seems more useful for cases where nvidia/amd support some of the
formats for buffer sharing..
Hm.. we've got physical layout, alignment restrictions, geometry
restrictions, what are the odds this will be shareable or compatible,
and how will the token names even looks when one puts all of this into them?
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On Mon, Feb 02, 2015 at 04:42:32PM +, Tvrtko Ursulin wrote:
>
> On 02/02/2015 04:32 PM, Rob Clark wrote:
> >On Mon, Feb 2, 2015 at 4:41 AM, Daniel Vetter wrote:
> >>On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
> >>>From: Tvrtko Ursulin
> >>>
> >>>To be used from the new addfb2 extension.
> >>>
> >>>Signed-off-by: Tvrtko Ursulin
> >>>---
> >>> include/uapi/drm/i915_drm.h | 13 +
> >>> 1 file changed, 13 insertions(+)
> >>>
> >>>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> >>>index 6eed16b..a7327fd 100644
> >>>--- a/include/uapi/drm/i915_drm.h
> >>>+++ b/include/uapi/drm/i915_drm.h
> >>>@@ -28,6 +28,7 @@
> >>> #define _UAPI_I915_DRM_H_
> >>>
> >>> #include
> >>>+#include
> >>>
> >>> /* Please note that modifications to all structs defined here are
> >>> * subject to backwards-compatibility constraints.
> >>>@@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
> >>> __u64 value;
> >>> };
> >>>
> >>>+/** @{
> >>>+ * Intel framebuffer modifiers
> >>>+ *
> >>>+ * Tiling modes supported by the display hardware
> >>>+ * to be passed in via the DRM addfb2 ioctl.
> >>>+ */
> >>>+/** None */
> >>>+#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL, 0x00L)
> >>>+/** X tiling */
> >>>+#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL,
> >>>0x01L)
> >>
> >>One thing I wonder here is whether we should have a modifier for each
> >>physical layout (tiling modes do change slightly between hw) or whether we
> >>should just continue to assume that this is Intel-specific and add a
> >>disclaimer that the precise layout depends upon the actual intel box
> >>you're running on?
> >
> >I'd kind of lean towards different modifiers per physical layout..
> >that seems more useful for cases where nvidia/amd support some of the
> >formats for buffer sharing..
>
> Hm.. we've got physical layout, alignment restrictions, geometry
> restrictions, what are the odds this will be shareable or compatible, and
> how will the token names even looks when one puts all of this into them?
On top of that there's a _lot_ of different physical layouts for just X
tiling. At least if you look at more than just modern platforms. And often
userspace doesn't even know which precise variant it is.
I think if we eventually have a match with some other vendor format (the
one with nvidia wasn't intentionally, it only works if you have swizzling
enabled, not without swizzling) then we could do some aliasing: Define a
new vendor neutral code which then all drivers supporting it would remap
to the correct internal/vendor-specific representation.
Of course integrated gpus are special, with plug-in pci devices you really
have to spec the full thing.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 4/6] drm/i915: Use framebuffer tiling mode for display purposes
On Mon, Feb 02, 2015 at 10:29:19AM +, Tvrtko Ursulin wrote:
>
> On 02/02/2015 09:49 AM, Daniel Vetter wrote:
> >On Fri, Jan 30, 2015 at 05:36:56PM +, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin
> >>
> >>To prepare for framebuffer modifiers, move tiling definition from the
> >>object into the framebuffer. Move in a way that framebuffer tiling is
> >>now used for display while object tiling remains for fencing.
> >>
> >>Signed-off-by: Tvrtko Ursulin
> >>---
> >> drivers/gpu/drm/i915/intel_display.c | 46
> >> +---
> >> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> >> drivers/gpu/drm/i915/intel_pm.c | 7 +++---
> >> drivers/gpu/drm/i915/intel_sprite.c | 26 ++--
> >> 4 files changed, 46 insertions(+), 35 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c
> >>b/drivers/gpu/drm/i915/intel_display.c
> >>index 4425e86..e22afbe 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -2211,7 +2211,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
> >>
> >>WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> >>
> >>- switch (obj->tiling_mode) {
> >>+ switch (to_intel_framebuffer(fb)->tiling_mode) {
> >>case I915_TILING_NONE:
> >
> >Imo we should just look at fb->modifier[0] and flip over all the enums. A
> >bit more invasive, but we also don't need to change all the platform code
> >at once - set_tiling already guarantees that no one can modify the bo
> >tiling mode when there's an fb object using it. Which means we can change
> >the code over at leasure.
>
> What do you mean by "flip over"?
>
> To make places which need to get fb tiling format use fb->modifier[0]
> directly rather than have them mapped to tiling enums at fb init?
Yes. That way we can drop intel_fb->tiling_mode and have one less somewhat
redundant thing to keep in sync with everything else. Hoping that everyone
just uses the same ime just doesn't work ;-)
> >>@@ -9764,6 +9768,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> >>work->event = event;
> >>work->crtc = crtc;
> >>work->old_fb_obj = intel_fb_obj(old_fb);
> >>+ work->old_tiling_mode = to_intel_framebuffer(old_fb)->tiling_mode;
> >
> >Hm, that's actually an interesting bugfix - currently userspace could be
> >sneaky and destroy the old fb immediately after the flip completes and the
> >change the tiling of the underlying object before the unpin work had a
> >chance to run (needs some fudgin with rt prios to starve workers to make
> >this work though).
> >
> >Imo the right fix is to hold a reference onto the fb and not the
> >underlying gem object. With that tiling is guaranteed not to change.
>
> Ok I'll pull it out in a separate patch.
>
> >>INIT_WORK(&work->work, intel_unpin_work_fn);
> >>
> >>ret = drm_crtc_vblank_get(crtc);
> >>@@ -9814,7 +9819,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> >>
> >>if (IS_VALLEYVIEW(dev)) {
> >>ring = &dev_priv->ring[BCS];
> >>- if (obj->tiling_mode != work->old_fb_obj->tiling_mode)
> >>+ if (to_intel_framebuffer(fb)->tiling_mode !=
> >>+ work->old_tiling_mode)
> >>/* vlv: DISPLAY_FLIP fails to change tiling */
> >>ring = NULL;
> >>} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> >>@@ -12190,7 +12196,8 @@ intel_check_cursor_plane(struct drm_plane *plane,
> >>
> >>/* we only need to pin inside GTT if cursor is non-phy */
> >>mutex_lock(&dev->struct_mutex);
> >>- if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) {
> >>+ if (!INTEL_INFO(dev)->cursor_needs_physical &&
> >>+ to_intel_framebuffer(fb)->tiling_mode) {
> >>DRM_DEBUG_KMS("cursor cannot be tiled\n");
> >>ret = -EINVAL;
> >>}
> >>@@ -12772,6 +12779,7 @@ static int intel_framebuffer_init(struct drm_device
> >>*dev,
> >>drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
> >>intel_fb->obj = obj;
> >>intel_fb->obj->framebuffer_references++;
> >>+ intel_fb->tiling_mode = obj->tiling_mode;
> >
> >One side-effect of using fb->modifier[0] is that if the modifier flag is
> >_not_ set, we need to reconstruct this field from obj->tiling_mode here.
> >Otoh if it is set this code here should check that fb->modifier and
> >obj->tiling_mode are consistent.
> >
> >Perhaps best to split this change out as a prep patch, like you've done
> >with the code for the initial framebuffer.
>
> If I understood correctly what you meant in the first quote then yes.
You've already done it, it's the next patch - I just read patch series
sequentially ;-) Patch needs a bit of adjustment ofc due to the removal of
intel_fb->tiling_mode, and needs to be before this one here so that
fb->modifier[0] is guaranteed to be valid also for legacy userspace.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://b
[Intel-gfx] [PATCH v2] drm/i915: Introduce intel_set_rps()
From: Ville Syrjälä
Replace the valleyview_set_rps() and gen6_set_rps() calls with
intel_set_rps() which itself does the IS_VALLEYVIEW() check. The
code becomes simpler since the callers don't have to do this check
themselves.
Most of the change was performe with the following semantic patch:
@@
expression E1, E2, E3;
@@
- if (IS_VALLEYVIEW(E1)) {
- valleyview_set_rps(E2, E3);
- } else {
- gen6_set_rps(E2, E3);
- }
+ intel_set_rps(E2, E3);
Adding intel_set_rps() and making valleyview_set_rps() and gen6_set_rps()
static was done manually. Also valleyview_set_rps() had to be moved a
bit avoid a forward declaration.
v2: Use a less greedy semantic patch
Cc: Chris Wilson
Suggested-by: Chris Wilson
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_debugfs.c | 10 ++-
drivers/gpu/drm/i915/i915_drv.h | 3 +--
drivers/gpu/drm/i915/i915_irq.c | 5 +---
drivers/gpu/drm/i915/i915_sysfs.c | 10 ++-
drivers/gpu/drm/i915/intel_pm.c | 53 -
5 files changed, 34 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index b315f01..ddb06e1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4168,10 +4168,7 @@ i915_max_freq_set(void *data, u64 val)
dev_priv->rps.max_freq_softlimit = val;
- if (IS_VALLEYVIEW(dev))
- valleyview_set_rps(dev, val);
- else
- gen6_set_rps(dev, val);
+ intel_set_rps(dev, val);
mutex_unlock(&dev_priv->rps.hw_lock);
@@ -4246,10 +4243,7 @@ i915_min_freq_set(void *data, u64 val)
dev_priv->rps.min_freq_softlimit = val;
- if (IS_VALLEYVIEW(dev))
- valleyview_set_rps(dev, val);
- else
- gen6_set_rps(dev, val);
+ intel_set_rps(dev, val);
mutex_unlock(&dev_priv->rps.hw_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4179b90..30a6ac6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3182,8 +3182,7 @@ extern void i915_redisable_vga(struct drm_device *dev);
extern void i915_redisable_vga_power_on(struct drm_device *dev);
extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
extern void intel_init_pch_refclk(struct drm_device *dev);
-extern void gen6_set_rps(struct drm_device *dev, u8 val);
-extern void valleyview_set_rps(struct drm_device *dev, u8 val);
+extern void intel_set_rps(struct drm_device *dev, u8 val);
extern void intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
bool enable);
extern void intel_detect_pch(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2399eae..2a8cb83 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1243,10 +1243,7 @@ static void gen6_pm_rps_work(struct work_struct *work)
dev_priv->rps.last_adj = new_delay - dev_priv->rps.cur_freq;
- if (IS_VALLEYVIEW(dev_priv->dev))
- valleyview_set_rps(dev_priv->dev, new_delay);
- else
- gen6_set_rps(dev_priv->dev, new_delay);
+ intel_set_rps(dev_priv->dev, new_delay);
mutex_unlock(&dev_priv->rps.hw_lock);
}
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index 49f5ade..cdc9da0 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -402,10 +402,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
/* We still need *_set_rps to process the new max_delay and
* update the interrupt limits and PMINTRMSK even though
* frequency request may be unchanged. */
- if (IS_VALLEYVIEW(dev))
- valleyview_set_rps(dev, val);
- else
- gen6_set_rps(dev, val);
+ intel_set_rps(dev, val);
mutex_unlock(&dev_priv->rps.hw_lock);
@@ -464,10 +461,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
/* We still need *_set_rps to process the new min_delay and
* update the interrupt limits and PMINTRMSK even though
* frequency request may be unchanged. */
- if (IS_VALLEYVIEW(dev))
- valleyview_set_rps(dev, val);
- else
- gen6_set_rps(dev, val);
+ intel_set_rps(dev, val);
mutex_unlock(&dev_priv->rps.hw_lock);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6ece663..bebefe7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3750,7 +3750,7 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private
*dev_priv, u8 val)
/* gen6_set_rps is called to update the frequency request, but should also be
* called when the range (min_delay and max_delay) is modified so that we can
* update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
-v
Re: [Intel-gfx] [RFC 5/6] drm/i915: Allow fb modifier to set framebuffer tiling
On Mon, Feb 02, 2015 at 10:36:30AM +, Tvrtko Ursulin wrote:
>
> On 02/02/2015 09:54 AM, Daniel Vetter wrote:
> >On Fri, Jan 30, 2015 at 05:36:57PM +, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin
> >>
> >>Use the fb modifier if it was specified over object tiling mode.
> >>
> >>Signed-off-by: Tvrtko Ursulin
> >>---
> >> drivers/gpu/drm/i915/intel_display.c | 40
> >> +---
> >> 1 file changed, 33 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c
> >>b/drivers/gpu/drm/i915/intel_display.c
> >>index e22afbe..ca69da0 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -12671,6 +12671,20 @@ static const struct drm_framebuffer_funcs
> >>intel_fb_funcs = {
> >>.create_handle = intel_user_framebuffer_create_handle,
> >> };
> >>
> >>+static unsigned int
> >>+intel_fb_modifier_to_tiling(u64 modifier)
> >>+{
> >>+ switch (modifier) {
> >>+ case I915_FORMAT_MOD_X_TILED:
> >>+ return I915_TILING_X;
> >>+ default:
> >>+ case I915_FORMAT_MOD_NONE:
> >>+ break;
> >>+ }
> >>+
> >>+ return I915_TILING_NONE;
> >>+}
> >>+
> >> static int intel_framebuffer_init(struct drm_device *dev,
> >> struct intel_framebuffer *intel_fb,
> >> struct drm_mode_fb_cmd2 *mode_cmd,
> >>@@ -12678,11 +12692,23 @@ static int intel_framebuffer_init(struct
> >>drm_device *dev,
> >> {
> >>int aligned_height;
> >>int pitch_limit;
> >>+ unsigned int tiling_mode = obj->tiling_mode;
> >>int ret;
> >>
> >>WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> >>
> >>- if (obj->tiling_mode == I915_TILING_Y) {
> >>+ if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> >>+ tiling_mode =
> >>+ intel_fb_modifier_to_tiling(mode_cmd->modifier[0]);
> >>+ if (tiling_mode != obj->tiling_mode &&
> >>+ obj->tiling_mode != I915_TILING_NONE) {
> >>+ DRM_ERROR("Tiling modifier mismatch %u vs obj %u!\n",
> >>+ tiling_mode, obj->tiling_mode);
> >>+ return -EINVAL;
> >>+ }
> >>+ }
> >
> >Ah, here comes the magic. I think this might be simpler if we just use
> >->modifier (and fix it up if FB_MODIFIERS isn't set).
> >
> >Btw another reason for this split is that this way we have a clear
> >separation between the tiling modes supported generally (as fb modifiers)
> >and the tiling modes supported by fences. It might therefore make sense to
> >rename obj->tiling_mode with a cocci patch to obj->fencing_mode or
> >->fence_tiling_mode). To make it really clear that it's just about the
> >global gtt fences and nothing more.
>
> I don't really like using ->modifier directly in tiling patch since it is an
> bag of unrelated stuff, not only a superset. Unrelated especially, but not
> only, from the point of view of call sites / users.
>
> Therefore I see some design elegance in extracting the tiling, or any other
> logical group of modifiers before hand.
>
> At the very least would call something like intel_fb_modifier_to_tiling(),
> but, it is very ugly to have a dynamic cost at every call site. Which is
> another reason why I preferred to extract the data before hand.
The reason is that the current tiling_mode enum is userspace ABI, and
it's just for how to fence global gtt mappings. That's the point of
splitting the fb modifiers out like in this rfc.
So if you add your fancy new tiling mode you can't do that, since you
can't extend the tiling_mode enum. Adding another enum also seems a bit
too much when we already have fb_modifiers.
And if fb_modifiers get too complicated we can add helper functions which
normalize stuff, e.g. extract just the base tiling mode and remove other
things (like compression mode or whatever it's going to be).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Ignore DRM_MODE_FLAG_DBLCLK flag with DP
From: Ville Syrjälä
The DP spec has nothing at all to say about double clocked modes. One
might assume they don't exist, and if you think about the concept
doesn't make much sense since the link already runs at higher fixed
frequency. So let's drop the DRM_MODE_FLAG_DBLCLK checks and simply
use the mode as if it was not double clocked.
I've tested this on a Dell UP2414Q which claims to support 720x576i
and 720x480i double clocked CEA modes, and it seems perfectly happy
with both modes using the 1x clock.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index eea9e36..4b0dadb 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -222,9 +222,6 @@ intel_dp_mode_valid(struct drm_connector *connector,
if (mode->clock < 1)
return MODE_CLOCK_LOW;
- if (mode->flags & DRM_MODE_FLAG_DBLCLK)
- return MODE_H_ILLEGAL;
-
return MODE_OK;
}
@@ -1189,9 +1186,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
intel_connector->panel.fitting_mode);
}
- if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
- return false;
-
DRM_DEBUG_KMS("DP link computation with max lane count %i "
"max bw %02x pixel clock %iKHz\n",
max_lane_count, bws[max_clock],
--
2.0.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Ignore DRM_MODE_FLAG_DBLCLK flag with DP
On Mon, Feb 02, 2015 at 07:16:33PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The DP spec has nothing at all to say about double clocked modes. One
> might assume they don't exist, and if you think about the concept
> doesn't make much sense since the link already runs at higher fixed
> frequency. So let's drop the DRM_MODE_FLAG_DBLCLK checks and simply
> use the mode as if it was not double clocked.
>
> I've tested this on a Dell UP2414Q which claims to support 720x576i
> and 720x480i double clocked CEA modes, and it seems perfectly happy
> with both modes using the 1x clock.
>
> Signed-off-by: Ville Syrjälä
What happens when this is for a dp->vga sst dongle? Or an active dvi/hdmi
dongle (not sure those exist ...), are we sure this wont upset an existing
screen somewhere?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_dp.c | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index eea9e36..4b0dadb 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -222,9 +222,6 @@ intel_dp_mode_valid(struct drm_connector *connector,
> if (mode->clock < 1)
> return MODE_CLOCK_LOW;
>
> - if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> - return MODE_H_ILLEGAL;
> -
> return MODE_OK;
> }
>
> @@ -1189,9 +1186,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>
> intel_connector->panel.fitting_mode);
> }
>
> - if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> - return false;
> -
> DRM_DEBUG_KMS("DP link computation with max lane count %i "
> "max bw %02x pixel clock %iKHz\n",
> max_lane_count, bws[max_clock],
> --
> 2.0.5
>
> ___
> 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 5/6] drm/i915: Allow fb modifier to set framebuffer tiling
On 02/02/2015 05:15 PM, Daniel Vetter wrote:
On Mon, Feb 02, 2015 at 10:36:30AM +, Tvrtko Ursulin wrote:
On 02/02/2015 09:54 AM, Daniel Vetter wrote:
On Fri, Jan 30, 2015 at 05:36:57PM +, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
Use the fb modifier if it was specified over object tiling mode.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/intel_display.c | 40 +---
1 file changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index e22afbe..ca69da0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12671,6 +12671,20 @@ static const struct drm_framebuffer_funcs
intel_fb_funcs = {
.create_handle = intel_user_framebuffer_create_handle,
};
+static unsigned int
+intel_fb_modifier_to_tiling(u64 modifier)
+{
+ switch (modifier) {
+ case I915_FORMAT_MOD_X_TILED:
+ return I915_TILING_X;
+ default:
+ case I915_FORMAT_MOD_NONE:
+ break;
+ }
+
+ return I915_TILING_NONE;
+}
+
static int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *intel_fb,
struct drm_mode_fb_cmd2 *mode_cmd,
@@ -12678,11 +12692,23 @@ static int intel_framebuffer_init(struct drm_device
*dev,
{
int aligned_height;
int pitch_limit;
+ unsigned int tiling_mode = obj->tiling_mode;
int ret;
WARN_ON(!mutex_is_locked(&dev->struct_mutex));
- if (obj->tiling_mode == I915_TILING_Y) {
+ if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
+ tiling_mode =
+ intel_fb_modifier_to_tiling(mode_cmd->modifier[0]);
+ if (tiling_mode != obj->tiling_mode &&
+ obj->tiling_mode != I915_TILING_NONE) {
+ DRM_ERROR("Tiling modifier mismatch %u vs obj %u!\n",
+ tiling_mode, obj->tiling_mode);
+ return -EINVAL;
+ }
+ }
Ah, here comes the magic. I think this might be simpler if we just use
->modifier (and fix it up if FB_MODIFIERS isn't set).
Btw another reason for this split is that this way we have a clear
separation between the tiling modes supported generally (as fb modifiers)
and the tiling modes supported by fences. It might therefore make sense to
rename obj->tiling_mode with a cocci patch to obj->fencing_mode or
->fence_tiling_mode). To make it really clear that it's just about the
global gtt fences and nothing more.
I don't really like using ->modifier directly in tiling patch since it is an
bag of unrelated stuff, not only a superset. Unrelated especially, but not
only, from the point of view of call sites / users.
Therefore I see some design elegance in extracting the tiling, or any other
logical group of modifiers before hand.
At the very least would call something like intel_fb_modifier_to_tiling(),
but, it is very ugly to have a dynamic cost at every call site. Which is
another reason why I preferred to extract the data before hand.
The reason is that the current tiling_mode enum is userspace ABI, and
it's just for how to fence global gtt mappings. That's the point of
splitting the fb modifiers out like in this rfc.
So if you add your fancy new tiling mode you can't do that, since you
can't extend the tiling_mode enum. Adding another enum also seems a bit
Why not? It is not changing the ABI since obj->tiling_mode stays exactly
the same as it is today.
Do you worry about leaking new data out in i915_drm.h, under the
I915_TILING_* #defines? I don't see that we have to change that at all.
too much when we already have fb_modifiers.
And if fb_modifiers get too complicated we can add helper functions which
normalize stuff, e.g. extract just the base tiling mode and remove other
things (like compression mode or whatever it's going to be).
So you are strongly for "looking into a bag of stuff" to see if anything
interesting is there on every call site?
Helper functions in my view only marginally help there - they make the
code neater but design is conceptually still untidy. And you add
pointless processing on every call site.
I just don't see what is the problem with extracting the interesting
data "from the bag" at fb init time. If you tried to make some
synchronization argument in the other reply I don't get it.
fb->modifier[0] should be, in my opinion, viewed as immutable. And it
lives at the base class level while in intel_frambuffer sub-class it
should be just fine to "parse" that into directly usable data stored at
the sub-class level.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Ignore DRM_MODE_FLAG_DBLCLK flag with DP
On Mon, Feb 02, 2015 at 06:21:31PM +0100, Daniel Vetter wrote:
> On Mon, Feb 02, 2015 at 07:16:33PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > The DP spec has nothing at all to say about double clocked modes. One
> > might assume they don't exist, and if you think about the concept
> > doesn't make much sense since the link already runs at higher fixed
> > frequency. So let's drop the DRM_MODE_FLAG_DBLCLK checks and simply
> > use the mode as if it was not double clocked.
> >
> > I've tested this on a Dell UP2414Q which claims to support 720x576i
> > and 720x480i double clocked CEA modes, and it seems perfectly happy
> > with both modes using the 1x clock.
> >
> > Signed-off-by: Ville Syrjälä
>
> What happens when this is for a dp->vga sst dongle? Or an active dvi/hdmi
> dongle (not sure those exist ...), are we sure this wont upset an existing
> screen somewhere?
If it's a native DP sink, I'd expect it to not advertize the mode if it
can't do it. Unless of course we're actually supposed to double clock
with DP. As stated I can't find anything in the spec about such things.
Also the specs are rather vague on what kind of infoframes you're
supposed to send DP sinks. If we're supposed to send the AVI infoframe
then we could send the VIC and the sink could know it needs to deal
with the doubled pixels, but othewise there's no way to tell the sink
about this using any native DP mechanism. I wish the DP spec would
be more clear on how it interacts with CEA-861, rather than just have
some vague references to it. I suppose I could try to enable the AVI
infoframe and see what happens...
As for dongles, VGA doesn't have a clock so I expect that should just
work (tm). For active DP->HDMI I suppose the dongle should realize it
needs to double up (assuming we're not supposed to double clock DP),
but I've not tried it. We do have one active dongle here so I should
be able to try it.
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index eea9e36..4b0dadb 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -222,9 +222,6 @@ intel_dp_mode_valid(struct drm_connector *connector,
> > if (mode->clock < 1)
> > return MODE_CLOCK_LOW;
> >
> > - if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> > - return MODE_H_ILLEGAL;
> > -
> > return MODE_OK;
> > }
> >
> > @@ -1189,9 +1186,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >
> > intel_connector->panel.fitting_mode);
> > }
> >
> > - if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> > - return false;
> > -
> > DRM_DEBUG_KMS("DP link computation with max lane count %i "
> > "max bw %02x pixel clock %iKHz\n",
> > max_lane_count, bws[max_clock],
> > --
> > 2.0.5
> >
> > ___
> > 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
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Multiple declarations for intel_fbc_enabled
On Mon, Feb 02, 2015 at 11:40:13AM -0500, Ed Maste wrote: > A FreeBSD developer discovered that intel_fbc_enabled has a > declaration in two headers: > > sys/dev/drm2/i915/i915_drv.h:extern bool intel_fbc_enabled(struct > drm_device *dev); > sys/dev/drm2/i915/intel_drv.h:extern bool intel_fbc_enabled(struct > drm_device *dev); > > We have a slightly older version of the i915 driver on FreeBSD, but I > see that this is still the case in Linux (although the "extern" has > been removed from one of them). Commit 85208be added the one in > intel_drv.h and didn't remove the existing one. Seems to be fixed already with commit 7ff0ebcc1e30e3216c8c62ee71f59ac830b10364 Author: Rodrigo Vivi Date: Mon Dec 8 14:09:10 2014 -0200 drm/i915: Move FBC stuff to intel_fbc.c which will be in 3.20. Thanks for reporting anyway. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/6] drm/i915: Add tiled framebuffer modifiers
On Mon, Feb 2, 2015 at 11:59 AM, Daniel Vetter wrote:
> On Mon, Feb 02, 2015 at 04:42:32PM +, Tvrtko Ursulin wrote:
>>
>> On 02/02/2015 04:32 PM, Rob Clark wrote:
>> >On Mon, Feb 2, 2015 at 4:41 AM, Daniel Vetter wrote:
>> >>On Fri, Jan 30, 2015 at 05:36:54PM +, Tvrtko Ursulin wrote:
>> >>>From: Tvrtko Ursulin
>> >>>
>> >>>To be used from the new addfb2 extension.
>> >>>
>> >>>Signed-off-by: Tvrtko Ursulin
>> >>>---
>> >>> include/uapi/drm/i915_drm.h | 13 +
>> >>> 1 file changed, 13 insertions(+)
>> >>>
>> >>>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> >>>index 6eed16b..a7327fd 100644
>> >>>--- a/include/uapi/drm/i915_drm.h
>> >>>+++ b/include/uapi/drm/i915_drm.h
>> >>>@@ -28,6 +28,7 @@
>> >>> #define _UAPI_I915_DRM_H_
>> >>>
>> >>> #include
>> >>>+#include
>> >>>
>> >>> /* Please note that modifications to all structs defined here are
>> >>> * subject to backwards-compatibility constraints.
>> >>>@@ -1101,4 +1102,16 @@ struct drm_i915_gem_context_param {
>> >>> __u64 value;
>> >>> };
>> >>>
>> >>>+/** @{
>> >>>+ * Intel framebuffer modifiers
>> >>>+ *
>> >>>+ * Tiling modes supported by the display hardware
>> >>>+ * to be passed in via the DRM addfb2 ioctl.
>> >>>+ */
>> >>>+/** None */
>> >>>+#define I915_FORMAT_MOD_NONE fourcc_mod_code(INTEL, 0x00L)
>> >>>+/** X tiling */
>> >>>+#define I915_FORMAT_MOD_X_TILED fourcc_mod_code(INTEL,
>> >>>0x01L)
>> >>
>> >>One thing I wonder here is whether we should have a modifier for each
>> >>physical layout (tiling modes do change slightly between hw) or whether we
>> >>should just continue to assume that this is Intel-specific and add a
>> >>disclaimer that the precise layout depends upon the actual intel box
>> >>you're running on?
>> >
>> >I'd kind of lean towards different modifiers per physical layout..
>> >that seems more useful for cases where nvidia/amd support some of the
>> >formats for buffer sharing..
>>
>> Hm.. we've got physical layout, alignment restrictions, geometry
>> restrictions, what are the odds this will be shareable or compatible, and
>> how will the token names even looks when one puts all of this into them?
>
> On top of that there's a _lot_ of different physical layouts for just X
> tiling. At least if you look at more than just modern platforms. And often
> userspace doesn't even know which precise variant it is.
hmm, if userspace doesn't know the format, that doesn't bode well for
sharing.. but in that case I915_FORMAT_MOD_DTRT alias might make
sense..
> I think if we eventually have a match with some other vendor format (the
> one with nvidia wasn't intentionally, it only works if you have swizzling
> enabled, not without swizzling) then we could do some aliasing: Define a
> new vendor neutral code which then all drivers supporting it would remap
> to the correct internal/vendor-specific representation.
>
> Of course integrated gpus are special, with plug-in pci devices you really
> have to spec the full thing.
the problem is if you are going to be sharing with another gpu, that
one is going to be plug-in ;-)
BR,
-R
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 5/6] drm/i915: Allow fb modifier to set framebuffer tiling
On Mon, Feb 02, 2015 at 05:30:36PM +, Tvrtko Ursulin wrote:
>
> On 02/02/2015 05:15 PM, Daniel Vetter wrote:
> >On Mon, Feb 02, 2015 at 10:36:30AM +, Tvrtko Ursulin wrote:
> >>
> >>On 02/02/2015 09:54 AM, Daniel Vetter wrote:
> >>>On Fri, Jan 30, 2015 at 05:36:57PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Use the fb modifier if it was specified over object tiling mode.
>
> Signed-off-by: Tvrtko Ursulin
> ---
> drivers/gpu/drm/i915/intel_display.c | 40
> +---
> 1 file changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e22afbe..ca69da0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12671,6 +12671,20 @@ static const struct drm_framebuffer_funcs
> intel_fb_funcs = {
> .create_handle = intel_user_framebuffer_create_handle,
> };
>
> +static unsigned int
> +intel_fb_modifier_to_tiling(u64 modifier)
> +{
> + switch (modifier) {
> + case I915_FORMAT_MOD_X_TILED:
> + return I915_TILING_X;
> + default:
> + case I915_FORMAT_MOD_NONE:
> + break;
> + }
> +
> + return I915_TILING_NONE;
> +}
> +
> static int intel_framebuffer_init(struct drm_device *dev,
> struct intel_framebuffer *intel_fb,
> struct drm_mode_fb_cmd2 *mode_cmd,
> @@ -12678,11 +12692,23 @@ static int intel_framebuffer_init(struct
> drm_device *dev,
> {
> int aligned_height;
> int pitch_limit;
> + unsigned int tiling_mode = obj->tiling_mode;
> int ret;
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
> - if (obj->tiling_mode == I915_TILING_Y) {
> + if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> + tiling_mode =
> + intel_fb_modifier_to_tiling(mode_cmd->modifier[0]);
> + if (tiling_mode != obj->tiling_mode &&
> + obj->tiling_mode != I915_TILING_NONE) {
> + DRM_ERROR("Tiling modifier mismatch %u vs obj %u!\n",
> + tiling_mode, obj->tiling_mode);
> + return -EINVAL;
> + }
> + }
> >>>
> >>>Ah, here comes the magic. I think this might be simpler if we just use
> >>>->modifier (and fix it up if FB_MODIFIERS isn't set).
> >>>
> >>>Btw another reason for this split is that this way we have a clear
> >>>separation between the tiling modes supported generally (as fb modifiers)
> >>>and the tiling modes supported by fences. It might therefore make sense to
> >>>rename obj->tiling_mode with a cocci patch to obj->fencing_mode or
> >>>->fence_tiling_mode). To make it really clear that it's just about the
> >>>global gtt fences and nothing more.
> >>
> >>I don't really like using ->modifier directly in tiling patch since it is an
> >>bag of unrelated stuff, not only a superset. Unrelated especially, but not
> >>only, from the point of view of call sites / users.
> >>
> >>Therefore I see some design elegance in extracting the tiling, or any other
> >>logical group of modifiers before hand.
> >>
> >>At the very least would call something like intel_fb_modifier_to_tiling(),
> >>but, it is very ugly to have a dynamic cost at every call site. Which is
> >>another reason why I preferred to extract the data before hand.
> >
> >The reason is that the current tiling_mode enum is userspace ABI, and
> >it's just for how to fence global gtt mappings. That's the point of
> >splitting the fb modifiers out like in this rfc.
> >
> >So if you add your fancy new tiling mode you can't do that, since you
> >can't extend the tiling_mode enum. Adding another enum also seems a bit
>
> Why not? It is not changing the ABI since obj->tiling_mode stays exactly the
> same as it is today.
>
> Do you worry about leaking new data out in i915_drm.h, under the
> I915_TILING_* #defines? I don't see that we have to change that at all.
I prefer to keep enums for different types of values separate to avoid
confusion.
> >too much when we already have fb_modifiers.
> >
> >And if fb_modifiers get too complicated we can add helper functions which
> >normalize stuff, e.g. extract just the base tiling mode and remove other
> >things (like compression mode or whatever it's going to be).
>
> So you are strongly for "looking into a bag of stuff" to see if anything
> interesting is there on every call site?
>
> Helper functions in my view only marginally help there - they make the code
> neater but design is conceptually still untidy. And you add pointless
> processing on every call site.
>
> I just don't see what is the problem with extracting the interesting data
> "from the bag" at fb init time. If
[Intel-gfx] [PATCH i-g-t] kms_cursor_crc: Kernel now checks for integer overflow
As of kernel commit
commit a679064a7e9e8799177a64a31668a34a1bc6a4f1
Author: Matt Roper
Date: Fri Jan 30 16:22:37 2015 -0800
drm/i915: Switch planes from transitional helpers to full atomic helpers
the kernel now checks for cursor coordinates that would result in
integer overflow and returns -ERANGE, similar to the checking that was
already done for other plane types. We update kms_cursor_crc here to
reflect this small behavior change:
* Check for success at extreme boundary conditions INT_MAX-{width,height}
rather than INT_MAX
* Add new check for success at SHRT_MAX; if the driver were to
internally use short values and overflow, we could have the cursor
reappear on the screen.
* Add a test for failure with proper error code at INT_MAX-{width,height}+1
Signed-off-by: Matt Roper
---
tests/kms_cursor_crc.c | 29 -
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index e1390a7..64fea12 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -139,6 +139,29 @@ static void do_single_test(data_t *data, int x, int y)
igt_assert(igt_crc_equal(&crc, &ref_crc));
}
+static void do_fail_test(data_t *data, int x, int y, int expect)
+{
+ igt_display_t *display = &data->display;
+ igt_plane_t *cursor;
+ cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb);
+ int ret;
+
+ igt_print_activity();
+
+ /* Hardware test */
+ igt_paint_test_pattern(cr, data->screenw, data->screenh);
+ cursor_enable(data);
+ cursor = igt_output_get_plane(data->output, IGT_PLANE_CURSOR);
+ igt_plane_set_position(cursor, x, y);
+ ret = igt_display_try_commit2(display, COMMIT_LEGACY);
+
+ igt_plane_set_position(cursor, 0, 0);
+ cursor_disable(data);
+ igt_display_commit(display);
+
+ igt_assert(ret == expect);
+}
+
static void do_test(data_t *data,
int left, int right, int top, int bottom)
{
@@ -201,7 +224,11 @@ static void test_crc_offscreen(data_t *data)
do_test(data, left - (cursor_w+512), right + (cursor_w+512), top -
(cursor_h+512), bottom + (cursor_h+512));
/* go nuts */
- do_test(data, INT_MIN, INT_MAX, INT_MIN, INT_MAX);
+ do_test(data, INT_MIN, INT_MAX - cursor_w, INT_MIN, INT_MAX - cursor_h);
+ do_test(data, SHRT_MIN, SHRT_MAX, SHRT_MIN, SHRT_MAX);
+
+ /* Make sure we get -ERANGE on integer overflow */
+ do_fail_test(data, INT_MAX - cursor_w + 1, INT_MAX - cursor_h + 1,
-ERANGE);
}
static void test_crc_sliding(data_t *data)
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Implementation of SKL display power well support
On Fri, 2015-01-16 at 15:57 +, Damien Lespiau wrote:
> From: Satheeshakrishna M
>
> This patch implements core logic of SKL display power well.
>
> v2: Addressed Imre's comments
> - Added respective DDIs under power well #1 and #2
> - Simplified repetitive code in power well programming
>
> v3: Implemented Imre's comments
> - Further simplified power well programming
> - Made sure that PW 1 is enabled prior to PW 2
>
> v4: Fix minor conflict with the the cherryview support (Damien)
>
> v5: Add the PLL power domain to the always on power well (Damien)
>
> v6: Disable BIOS power well (Imre)
> Use power well data for comparison (Imre)
> Put the PLL power domain into PW1 as its needed for CDCLK (Satheesh,
> Damien)
>
> v7: Addressed Imre's comments
> - Lowered the time out to 1ms
> - Added parantheses in macro
> - Moved debug message and fixed wait_for interval
>
> v8:
> - Add a WARN() when swiching on an unknown power well (Imre, done by Damien)
> - Whitespace fixes (spaces instead of tabs) (Damien)
>
> v9: (Imre, done by Damien)
> - Merge the register definitions with this patch
> - Merge the MISC IO power well in this patch
>
> Signed-off-by: Satheeshakrishna M (v3,v6,v7)
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/i915_reg.h | 20 +++
> drivers/gpu/drm/i915/intel_runtime_pm.c | 230
>
> 2 files changed, 250 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a39bb03..cb96041 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -586,6 +586,19 @@ enum punit_power_well {
> PUNIT_POWER_WELL_NUM,
> };
>
> +enum skl_disp_power_wells {
> + SKL_DISP_PW_MISC_IO,
> + SKL_DISP_PW_DDI_A_E,
> + SKL_DISP_PW_DDI_B,
> + SKL_DISP_PW_DDI_C,
> + SKL_DISP_PW_DDI_D,
> + SKL_DISP_PW_1 = 14,
> + SKL_DISP_PW_2,
> +};
> +
> +#define SKL_POWER_WELL_STATE(pw) (1 << ((pw) * 2))
> +#define SKL_POWER_WELL_REQ(pw) (1 << (((pw) * 2) + 1))
> +
> #define PUNIT_REG_PWRGT_CTRL 0x60
> #define PUNIT_REG_PWRGT_STATUS 0x61
> #define PUNIT_PWRGT_MASK(power_well) (3 << ((power_well) *
> 2))
> @@ -6323,6 +6336,13 @@ enum punit_power_well {
> #define HSW_PWR_WELL_FORCE_ON (1<<19)
> #define HSW_PWR_WELL_CTL60x45414
>
> +/* SKL Fuse Status */
> +#define SKL_FUSE_STATUS 0x42000
> +#define SKL_FUSE_DOWNLOAD_STATUS (1<<31)
> +#define SKL_FUSE_PG0_DIST_STATUS (1<<27)
> +#define SKL_FUSE_PG1_DIST_STATUS (1<<26)
> +#define SKL_FUSE_PG2_DIST_STATUS (1<<25)
> +
> /* Per-pipe DDI Function Control */
> #define TRANS_DDI_FUNC_CTL_A 0x60400
> #define TRANS_DDI_FUNC_CTL_B 0x61400
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 49695d7..d72ec13 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -230,6 +230,146 @@ static void hsw_set_power_well(struct drm_i915_private
> *dev_priv,
> }
> }
>
> +#define SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS ( \
> + BIT(POWER_DOMAIN_PIPE_B) | \
> + BIT(POWER_DOMAIN_TRANSCODER_B) |\
> + BIT(POWER_DOMAIN_PIPE_C) | \
> + BIT(POWER_DOMAIN_TRANSCODER_C) |\
> + BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) | \
> + BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) | \
> + BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) | \
> + BIT(POWER_DOMAIN_AUX_B) | \
> + BIT(POWER_DOMAIN_AUX_C) | \
> + BIT(POWER_DOMAIN_AUX_D) | \
> + BIT(POWER_DOMAIN_AUDIO) | \
> + BIT(POWER_DOMAIN_INIT))
What about transcoder A? It's also on power well 2, and I can't see
anything preventing us to use DP or HDMI with it, so I think it needs to
be added here.
POWER_DOMAIN_VGA needs to be added here too.
> +#define SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS ( \
> + SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS | \
> + BIT(POWER_DOMAIN_PLLS) |\
> + BIT(POWER_DOMAIN_PIPE_A) | \
> + BIT(POWER_DOMAIN_TRANSCODER_EDP) | \
> + BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) | \
> + BIT(POWER_DOMAIN_PORT_DDI_A_2_LANES) | \
> + BIT(POWER_DOMAIN_PORT_DDI_A_4_LANES) | \
> + BIT(POWER_DOMAIN
Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5678 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -8 353/353 345/353 ILK 355/355 355/355 SNB 400/422 400/422 IVB +2 485/487 487/487 BYT 296/296 296/296 HSW +1 507/508 508/508 BDW 401/402 401/402 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gem_concurrent_blit_gpuX-bcs-gpu-read-after-write-interruptible PASS(2, M25M7) FAIL(1, M7) *PNV igt_gem_concurrent_blit_prw-rcs-early-read-interruptible PASS(2, M25M7) FAIL(1, M7) *PNV igt_gem_userptr_blits_coherency-sync PASS(2, M25M7) CRASH(1, M7) *PNV igt_gem_userptr_blits_coherency-unsync PASS(2, M25M7) NRUN(1, M7) *PNV igt_gen3_render_linear_blits PASS(6, M25M23M7) FAIL(1, M7) *PNV igt_gen3_render_mixed_blits PASS(2, M25M7) FAIL(1, M7) *PNV igt_gen3_render_tiledx_blits PASS(2, M25M7) FAIL(1, M7) *PNV igt_gen3_render_tiledy_blits PASS(2, M25M7) FAIL(1, M7) IVB igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(8, M34M21)PASS(10, M4M34) PASS(1, M4) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(7, M34M4M21)PASS(18, M34M4M21) PASS(1, M4) HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(2, M40)PASS(22, M40M20) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix frontbuffer false positve.
frontbuffer bits must be updated during commit times not on atomica prepare
one, otherwise we have a risk of false positive.
Cc Daniel Vetter
Cc: Sonika Jindal
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_display.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 73b5923..1e83124 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12176,9 +12176,6 @@ finish:
if (intel_crtc->active) {
if (intel_crtc->cursor_width != state->base.crtc_w)
intel_crtc->atomic.update_wm = true;
-
- intel_crtc->atomic.fb_bits |=
- INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
}
return ret;
@@ -12220,8 +12217,11 @@ update:
intel_crtc->cursor_width = state->base.crtc_w;
intel_crtc->cursor_height = state->base.crtc_h;
- if (intel_crtc->active)
+ if (intel_crtc->active) {
intel_crtc_update_cursor(crtc, state->visible);
+ intel_crtc->atomic.fb_bits |=
+ INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+ }
}
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Thu, Jan 29, 2015 at 08:56:06PM -0500, Michael Auchter wrote: > On Thu, Jan 29, 2015 at 06:12:31PM +0100, Daniel Vetter wrote: > > On Wed, Jan 28, 2015 at 10:36:02PM -0800, O'Rourke, Tom wrote: > > > On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote: > > > > On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote: > > > > > On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote: > > > > > > Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) > > > > > > resulted in > > > > > > this WARN at boot (and pretty frequently afterwards): > > > > > > > > > > > > WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 > > > > > > gen6_set_rps+0x371/0x3c0() > > > > > > WARN_ON(val > dev_priv->rps.max_freq_softlimit) > > > > > > > > > > [snip] > > > > > > > > > > > I'm not at all familiar with this hardware, but I took a quick look > > > > > > into > > > > > > what changed with this commit for my laptop. Before the commit, > > > > > > rps.min_freq_softlimit is 4 (from rps.min_freq) and > > > > > > rps.max_freq_softlimit is 22. > > > > > > > > > > > > After the commit, rps.min_freq_softlimit is set to the > > > > > > rps.efficient_freq value read from pcode, which is 34 on my laptop. > > > > > > So later when gen6_set_rps() is called with rps.min_freq_softlimit > > > > > > that > > > > > > warning is hit. > > > > > > > > > > > > Any thoughts? It certainly seems fishy that this commit causes > > > > > > rps.min_freq_softlimit to be greater than rps.max_freq_softlimit. > > > > > > > > > > Very fishy indeed. Moral of this story, never trust hw. > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > > b/drivers/gpu/drm/i915/intel_pm.c > > > > > index 3e630feb18e4..bbedd2901c54 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > > @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct > > > > > drm_device *dev) > > > > > &ddcc_status); > > > > > if (0 == ret) > > > > > dev_priv->rps.efficient_freq = > > > > > - (ddcc_status >> 8) & 0xff; > > > > > + clamp_t(u8, > > > > > + (ddcc_status >> 8) & 0xff, > > > > > + dev_priv->rps.min_freq, > > > > > + dev_priv->rps.max_freq); > > > > > > > > Maybe better to fall back to rp1_freq if this is bogus? > > > > > > > [TOR:] Michael, Thank you for bringing this problem to our attention. > > > > > > Yes, this function needs some range checking to maintain > > > RPn <= RPe <= RP0. > > > > > > A value of 34 seems too high for RPe. > > > What values does the Carbon X1 (Haswell) have for RPn and RP0? > > > > 4 & 22, already in Micheal's original bug report. > > > > Tom, can you pls polish the clamping into a proper patch with m-l > > references? [TOR:] Yes, I will submit a polished patch with proper range checking. > > > > Micheal, can you please test the first hunk from Chris (the one that adds > > the clamp) to make sure it does indeed address the WARN_ON you're seeing? > > The clamp suggested by Chris does indeed fix the WARN_ON. > > In the case where RPe is greater than RP0, RPe will now be clamped to > RP0. Is this likely to result in increased power consumption? > > At a quick glance on my laptop it does not (idling around 5W before and > after) but Ville had suggested earlier to fall back to RP1, which would > be more consistent with previous kernels. > > Thanks again for the quick responses, > Michael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Track old framebuffer instead of object
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5695 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 353/353 353/353 ILK -1 344/344 343/344 SNB 400/422 400/422 IVB +1-1 485/487 485/487 BYT 296/296 296/296 HSW +1-1 507/508 507/508 BDW 401/402 401/402 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt_kms_flip_flip-vs-panning PASS(2, M26) TIMEOUT(1, M26) IVB igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(8, M34M21)PASS(10, M4M34) PASS(1, M34) IVB igt_gem_storedw_batches_loop_secure-dispatch DMESG_WARN(1, M34)PASS(8, M34M4) DMESG_WARN(1, M34) *HSW igt_gem_pwrite_pread_display-copy-performance PASS(6, M40M20) DMESG_WARN(1, M40) HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(2, M40)PASS(22, M40M20) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Ignore DRM_MODE_FLAG_DBLCLK flag with DP
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5698 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -1 353/353 352/353 ILK 200/200 200/200 SNB 400/422 400/422 IVB +1 485/487 486/487 BYT 296/296 296/296 HSW 507/508 507/508 BDW -1 401/402 400/402 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gen3_render_linear_blits PASS(6, M25M23M7) CRASH(1, M23) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(7, M34M4M21)PASS(18, M34M4M21) PASS(1, M21) BDW igt_gem_pwrite_pread_uncached-pwrite-blt-gtt_mmap-performance DMESG_WARN(2, M28)PASS(7, M30M28) DMESG_WARN(1, M28) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v3 1/4] drm: Add support to find drm_panel by name
On 01/27/2015 03:01 PM, Shobhit Kumar wrote:
> For scenarios where OF is not available, we can use panel identification by
> name.
Any body had a look at this ?
Regards
Shobhit
>
> Signed-off-by: Shobhit Kumar
> ---
> drivers/gpu/drm/drm_panel.c | 18 ++
> include/drm/drm_panel.h | 3 +++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 2ef988e..e1cb8cf 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -95,6 +95,24 @@ struct drm_panel *of_drm_find_panel(struct device_node *np)
> EXPORT_SYMBOL(of_drm_find_panel);
> #endif
>
> +struct drm_panel *drm_find_panel_by_name(const char *name)
> +{
> + struct drm_panel *panel;
> +
> + mutex_lock(&panel_lock);
> +
> + list_for_each_entry(panel, &panel_list, list) {
> + if (strcmp(panel->name, name) == 0) {
> + mutex_unlock(&panel_lock);
> + return panel;
> + }
> + }
> +
> + mutex_unlock(&panel_lock);
> + return NULL;
> +}
> +EXPORT_SYMBOL(drm_find_panel_by_name);
> +
> MODULE_AUTHOR("Thierry Reding ");
> MODULE_DESCRIPTION("DRM panel infrastructure");
> MODULE_LICENSE("GPL and additional rights");
> diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
> index 1fbcc96..1ef9ff3 100644
> --- a/include/drm/drm_panel.h
> +++ b/include/drm/drm_panel.h
> @@ -74,6 +74,7 @@ struct drm_panel {
> struct drm_device *drm;
> struct drm_connector *connector;
> struct device *dev;
> + char name[NAME_MAX];
>
> const struct drm_panel_funcs *funcs;
>
> @@ -137,4 +138,6 @@ static inline struct drm_panel *of_drm_find_panel(struct
> device_node *np)
> }
> #endif
>
> +struct drm_panel *drm_find_panel_by_name(const char *name);
> +
> #endif
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Correct the variable holding the value for EOT to write
This isuue got introduced in - commit 24ee0e64909bf7f1953d87d3e1e29d93eafcad73 Author: Gaurav K Singh Date: Fri Dec 5 14:24:21 2014 +0530 drm/i915: Update the DSI enable path to support dual Signed-off-by: Shobhit Kumar --- drivers/gpu/drm/i915/intel_dsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index ef3df5e..6ce9c45 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -855,7 +855,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder) /* recovery disables */ - I915_WRITE(MIPI_EOT_DISABLE(port), val); + I915_WRITE(MIPI_EOT_DISABLE(port), tmp); /* in terms of low power clock */ I915_WRITE(MIPI_INIT_COUNT(port), intel_dsi->init_count); -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix frontbuffer false positve.
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5699 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 353/353 353/353 ILK 353/353 353/353 SNB +1 400/422 401/422 IVB +1-1 485/487 485/487 BYT 296/296 296/296 HSW +1-1 507/508 507/508 BDW 401/402 401/402 -Detailed- Platform Testdrm-intel-nightly Series Applied *SNB igt_kms_flip_event_leak NSPT(6, M35M22) PASS(1, M35) IVB igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(9, M34M21)PASS(10, M4M34) PASS(1, M34) IVB igt_gem_storedw_batches_loop_secure-dispatch DMESG_WARN(1, M34)PASS(9, M34M4) DMESG_WARN(1, M34) *HSW igt_gem_pwrite_pread_display-copy-performance PASS(7, M40M20) DMESG_WARN(1, M40) HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(2, M40)PASS(23, M40M20) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
