Commit Graph

82 Commits

Author SHA1 Message Date
Daniel Vetter 967667fd6d drm/plane-helper: Fixup mismerge
I somehow manage to screw up applying Laurent's patch in eca93e28c256:
"drm: Check in setcrtc if the primary plane supports the fb pixel
format". It was a conflict with

commit 3461b30b3e
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Mar 5 10:32:44 2015 +0100

    drm/plane-helper: unexport drm_primary_helper_create_plane

and I just didn't check that the solution from wiggle made sense.

Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: laurent.pinchart@ideasonboard.com
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: laurent.pinchart@ideasonboard.com
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
2015-03-11 16:43:40 +01:00
Laurent Pinchart 7eb5f302bb drm: Check in setcrtc if the primary plane supports the fb pixel format
Drivers implementing the universal planes API report the list of
supported pixel formats for the primary plane. Make sure the fb passed
to the setcrtc ioctl is compatible.

Drivers not implementing the universal planes API will have no format
reported for the primary plane, skip the check in that case.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2015-03-10 09:59:36 +01:00
Daniel Vetter 3461b30b3e drm/plane-helper: unexport drm_primary_helper_create_plane
We shouldn't tempt driver writers into using this since it uses a
default format list which is likely wrong. And when that's done we can
simplify the code a bit, too.

Noticed while reviewing a patch from Laurent.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
2015-03-10 09:59:35 +01:00
Tvrtko Ursulin d136dfeec8 drm: Pass in new and old plane state to prepare_fb and cleanup_fb
Use cases like rotation require these hooks to have some context so they
know how to prepare and cleanup the frame buffer correctly.

For i915 specifically, object backing pages need to be mapped differently
for different rotation modes and the driver needs to know which mapping to
instantiate and which to tear down when transitioning between them.

v2: Made passed in states const. (Daniel Vetter)

[airlied: add mdp5 and atmel fixups]
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
2015-03-05 09:49:02 +10:00
Dave Airlie 21773f16f2 Merge tag 'topic/atomic-core-2015-01-27' of git://anongit.freedesktop.org/drm-intel into drm-next
* tag 'topic/atomic-core-2015-01-27' of git://anongit.freedesktop.org/drm-intel:
  drm/atomic: Fix potential use of state after free
  drm/atomic-helper: debug output for modesets
  drm/atomic-helpers: Saner encoder/crtc callbacks
  drm/atomic-helpers: Recover full cursor plane behaviour
  drm/atomic-helper: add connector->dpms() implementation
  drm/atomic: Add drm_crtc_state->active
  drm: Add standardized boolean props
  drm/plane-helper: Fix transitional helper kerneldocs
  drm/plane-helper: Skip prepare_fb/cleanup_fb when newfb==oldfb

Conflicts:
	include/drm/drm_crtc_helper.h
2015-01-28 09:34:27 +10:00
Thierry Reding 407b8bd9f5 drm/plane: Add optional ->atomic_disable() callback
In order to prevent drivers from having to perform the same checks over
and over again, add an optional ->atomic_disable callback which the core
calls under the right circumstances.

v2: pass old state and detect edges to avoid calling ->atomic_disable on
already disabled planes, remove redundant comment (Daniel Vetter)

v3: rename helper to drm_atomic_plane_disabling() to clarify that it is
checking for transitions, move helper to drm_atomic_helper.h, clarify
check for !old_state and its relation to transitional helpers

Here's an extract from some discussion rationalizing the behaviour (for
a full version, see the reference below):

    > > Hm, thinking about this some more this will result in a slight difference
    > > in behaviour, at least when drivers just use the helper ->reset functions
    > > but don't disable everything:
    > > - With transitional helpers we assume we know nothing and call
    > >   ->atomic_disable.
    > > - With atomic old_state->crtc == NULL in the same situation right after
    > >   boot-up, but we asssume the plane is really off and _dont_ call
    > >   ->atomic_disable.
    > >
    > > Should we instead check for (old_state && old_state->crtc) and state that
    > > drivers need to make sure they don't have stuff hanging around?
    >
    > I don't think we can check for old_state because otherwise this will
    > always return false, whereas we really want it to force-disable planes
    > that could be on (lacking any more accurate information). For
    > transitional helpers anyway.
    >
    > For the atomic helpers, old_state will never be NULL, but I'd assume
    > that the driver would reconstruct the current state in ->reset().

    By the way, the reason for why old_state can be NULL with transitional
    helpers is the ordering of the steps in the atomic transition. Currently
    the Tegra patches do this (based on your blog post and the Exynos proto-
    type):

        1) atomic conversion, phase 1:
           - implement ->atomic_{check,update,disable}()
           - use drm_plane_helper_{update,disable}()

        2) atomic conversion, phase 2:
           - call drm_mode_config_reset() from ->load()
           - implement ->reset()

    That's only a partial list of what's done in these steps, but that's the
    only relevant pieces for why old_state is NULL.

    What happens is that without ->reset() implemented there won't be any
    initial state, hence plane->state (the old_state here) will be NULL the
    first time atomic state is applied.

    We could of course reorder the sequence such that drivers are required
    to hook up ->reset() before they can (or at the same as they) hook up
    the transitional helpers. We could add an appropriate WARN_ON to this
    helper to make that more obvious.

    However, that will not solve the problem because it only gets rid of the
    special case. We still don't know whether old_state->crtc == NULL is the
    current state or just the initial default.

    So no matter which way we do this, I don't see a way to get away without
    requiring specific semantics from drivers. They would be that:

        - drivers recreate the correct state in ->reset() so that
          old_state->crtc != NULL if the plane is really enabled

    or

        - drivers have to ensure that the real state in fact mirrors the
          initial default as encoded in the state (plane disabled)

References: http://lists.freedesktop.org/archives/dri-devel/2015-January/075578.html
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Thierry Reding <treding@nvidia.com>
2015-01-27 10:14:42 +01:00
Matt Roper 6a425c2a9b drm/plane-helper: Fix transitional helper kerneldocs
drm_plane_helper_{update,disable} are not specific to primary planes;
fix some copy/paste summaries to avoid confusion.

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2015-01-26 08:21:57 +01:00
Matt Roper 9289058362 drm/plane-helper: Skip prepare_fb/cleanup_fb when newfb==oldfb
When commiting a plane update where the framebuffer doesn't change, we
can skip the prepare_fb/cleanup_fb steps.  This also allows us to avoid
an unnecessary vblank wait at the end of the operation when we're just
moving a plane and not changing its image (e.g., for a cursor).

At the moment, i915 is the only upstream driver using the transitional
plane helpers, and thus the only driver affected by this change.

Note that this replicates a corresponding change in the atomic helpers
implemented in

commit ab58e3384b
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Nov 24 20:42:42 2014 +0100

    drm/atomic-helper: Skip vblank waits for unchanged fbs

Reported-by: Jeremiah Mahler <jmmahler@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88540
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Tested-by: Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2015-01-26 08:21:56 +01:00
Daniel Vetter 72a3697097 Merge branch 'topic/core-stuff' into topic/atomic-core
Backmerge my drm-misc branch because of conflicts. Just simple stuff
but better to clear this out before I merge the other atomic patches.

Conflicts:
drivers/gpu/drm/drm_crtc.c
drivers/gpu/drm/drm_edid.c

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
2014-12-17 20:24:02 +01:00
Daniel Vetter 07cc0ef67f drm/atomic: Introduce state->obj backpointers
Useful since this way we can pass around just the state objects and
will get ther real object, too.

Specifically this allows us to again simplify the parameters for
set_crtc_for_plane.

v2: msm already has it's own specific plane_reset hook, don't forget
that one!

v3: Fixup kerneldoc, reported by 0-day builder.

Cc: Rob Clark <robdclark@gmail.com>
Reviewed-by: Rob Clark <robdclark@gmail.com> (v2)
Tested-by: Rob Clark <robdclark@gmail.com> (v2)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
2014-12-17 20:23:23 +01:00
Matt Roper 7432ca5ace drm/plane-helper: Test for plane disable earlier
drm_plane_helper_check_update() currently uses crtc before testing whether
we're disabling the plane (fb == NULL).  Move the fb test before the first crtc
usage so that crtc == NULL doesn't have to be handled by the caller.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-12-11 16:57:18 +01:00
Thierry Reding f1c37e1adc drm/plane: Pass old state to ->atomic_update()
In most situations it will be useful to have the old state passed to the
->atomic_update() callback. For example if a plane is being disabled the
new state's .crtc field will be NULL, but some drivers may rely on this
field to program the CRTCs registers.

v2: rename variable to old_plane_state and remove redundant comment as
suggested by Daniel Vetter, remove an Exynos hunk that doesn't apply to
drm-next and add a hunk for pending MSM mdp5 changes

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-11-25 13:27:58 +01:00
Daniel Vetter eb84f976c8 Merge remote-tracking branch 'airlied/drm-next' into HEAD
Backmerge drm-next so that I can keep merging patches. Specifically I
want:
- atomic stuff, yay!
- eld parsing patch from Jani.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
2014-11-10 10:55:35 +01:00
Daniel Vetter 321ebf04dc drm/atomic: Refcounting for plane_state->fb
So my original plan was that the drm core refcounts framebuffers like
with the legacy ioctls. But that doesn't work for a bunch of reasons:

- State objects might live longer than until the next fb change
  happens for a plane. For example delayed cleanup work only happens
  _after_ the pageflip ioctl has completed. So this definitely doesn't
  work without the plane state holding its own references.

- The other issue is transition from legacy to atomic implementations,
  where the driver works under a mix of both worlds. Which means
  legacy paths might not properly update the ->fb pointer under
  plane->state->fb. Which is a bit a problem when then someone comes
  around and _does_ try to clean it up when it's long gone.

The second issue is just a bit a transition bug, since drivers should
update plane->state->fb in all the paths that aren't converted yet.
But a bit more robustness for the transition can't hurt - we pull
similar tricks with cleaning up the old fb in the transitional helpers
already.

The pattern for drivers that transition is

	if (plane->state)
		drm_atomic_set_fb_for_plane(plane->state, plane->fb);

inserted after the fb update has logically completed at the end of
->set_config (or ->set_base/mode_set if using the crtc helpers),
->page_flip, ->update_plane or any other entry point which updates
plane->fb.

v2: Update kerneldoc - copypasta fail.

v3: Fix spelling in the commit message (Sean).

Reviewed-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
2014-11-06 21:08:37 +01:00
Daniel Vetter 3150c7d0c6 drm: Docbook integration and over sections for all the new helpers
In all cases the text requires that new drivers are converted to the
atomic interfaces.

v2: Add overview for state handling.

v3: Review from Sean: Some spelling fixes and drop the misguided
hunk to remove rgba8888 from the plane helpers compat list.

Cc: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-11-06 21:08:32 +01:00
Daniel Vetter 2f324b42b7 drm/crtc-helper: Transitional functions using atomic plane helpers
These two functions allow drivers to reuse their atomic plane helpers
functions for the primary plane to implement the interfaces required
by the crtc helpers for the legacy ->set_config callback.

This is purely transitional and won't be used once the driver is fully
converted. But it allows partial conversions to the atomic plane
helpers which are functional.

v2:
- Use ->atomic_duplicate_state if available.
- Don't forget to run crtc_funcs->atomic_check.

v3: Shift source coordinates correctly for 16.16 fixed point.

v4: Don't forget to call ->atomic_destroy_state if available.

v5: Fixup kerneldoc.

v6: Reuse the plane_commit function from the transitional plane
helpers to avoid too much duplication.

v7:
- Remove some stale comment.
- Correctly handle the lack of plane->state object, necessary for
  transitional use.

v8: Fixup an embarrassing h/vdisplay mixup.

Reviewed-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-11-05 18:44:59 +01:00
Daniel Vetter acf24a395c drm/plane-helper: transitional atomic plane helpers
Converting a driver to the atomic interface can be a daunting
undertaking. One of the prerequisites is to have full universal planes
support.

To make that transition a bit easier this patch provides plane helpers
which use the new atomic helper callbacks just only for the plane
changes. This way the plane update functionality can be tested without
being forced to convert everything at once.

Of course a real atomic update capable driver will implement the
all plane properties through the atomic interface, so these helpers
are mostly transitional. But they can be used to enable proper
universal plane support, especially once the crtc helpers have also
been adapted.

v2: Use ->atomic_duplicate_state if available.

v3: Don't forget to call ->atomic_destroy_state if available.

v4: Fixup kerneldoc, reported by Paulo.

v5: Extract a common plane_commit helper and fix some bugs in the
plane_state setup of the plane_disable implementation.

v6: Fix issues with the cleanup of the old fb. Since transitional
helpers can be mixed we need to assume that the old fb has been set up
by a legacy path (e.g. set_config or page_flip when the primary plane
is converted to use these functions already). Hence pass an additional
old_fb parameter to plane_commit to do that cleanup work correctly.

v7:
- Fix spurious WARNING (crtc helpers really love to disable stuff
  harder) and fix array index bonghits.
- Correctly handle the lack of plane->state object, necessary for
  transitional use.
- Don't indicate failure if drm_vblank_get doesn't work - that's
  expected when the pipe is in dpms off mode.

v8: Review from Sean:
- s/fail/out/ to make the meaning of a label more clear.
- spelling fix in the commit message.

Cc: Paulo Zanoni <przanoni@gmail.com>
Cc: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-11-05 18:07:01 +01:00
Gustavo Padovan 083fe3b035 drm: make sure visible is set to false if fb is null
We can't let visible set true while the fb is null, some places of
the code only check for visible to base its decisions.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-11-04 23:21:29 +01:00
Matt Roper 3281cc7e34 drm/plane-helper: Use proper plane init function
drm_plane_init() (the legacy plane initialization function) takes a bool
as its final parameter; originally this indicated whether a plane was
'private' to the driver (before the DRM core understood non-overlay
planes), now it indicates whether the plane is a primary plane (private
planes were used by some drivers to represent primary planes
internally).  The newer drm_universal_plane_init() take an 'enum
drm_plane_type' as its final parameter to allow the caller to specify
the specific plane type desired (primary, cursor, or overlay).

Due to a rebasing mistake, the primary plane helper is currently passing
DRM_PLANE_TYPE_PRIMARY (enum value = 1) for drm_plane_init()'s boolean
'is_primary' parameter.  This winds up giving the correct behavior since
DRM_PLANE_TYPE_PRIMARY evaluates as true, but is confusing to anyone
reading the code since we're passing an enum value (one of three
possible values) for a boolean parameter.

Replace the primary plane helper's call to drm_plane_init() with
drm_universal_plane_init() so that the parameter and value types match
up as expected.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-07-18 15:39:28 +02:00
Matt Roper 7daf8d54c1 drm/plane-helper: Add drm_plane_helper_check_update() (v3)
Pull the parameter checking from drm_primary_helper_update() out into
its own function; drivers that provide their own setplane()
implementations rather than using the helper may still want to share
this parameter checking logic.

A few of the checks here were also updated based on suggestions by
Ville Syrjälä.

v3:
 - s/primary_helper/plane_helper/ --- this checking logic may be useful
   for other types of planes as well.
 - Fix visibility check (need to dereference visibility pointer)
v2:
 - Pass src/dest/clip rects and min/max scaling down to helper to avoid
   duplication of effort between helper and drivers (suggested by
   Ville).
 - Allow caller to specify whether the primary plane should be
   updatable while the crtc is disabled.

Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Chon Ming Lee <chon.ming.lee@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
[danvet: Include header properly and fixup declaration mismatch to
make this compile.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-06-05 08:52:43 +02:00
Matt Roper 7f994f3fc4 drm: Check CRTC compatibility in setplane
The DRM core setplane code should check that the plane is usable on the
specified CRTC before calling into the driver.

Prior to this patch, a plane's possible_crtcs field was purely
informational for userspace and was never actually verified at the
kernel level (aside from the primary plane helper).

Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Chon Ming Lee <chon.ming.lee@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-06-05 08:52:43 +02:00
Rob Clark 51fd371bba drm: convert crtc and connection_mutex to ww_mutex (v5)
For atomic, it will be quite necessary to not need to care so much
about locking order.  And 'state' gives us a convenient place to stash a
ww_ctx for any sort of update that needs to grab multiple crtc locks.

Because we will want to eventually make locking even more fine grained
(giving locks to planes, connectors, etc), split out drm_modeset_lock
and drm_modeset_acquire_ctx to track acquired locks.

Atomic will use this to keep track of which locks have been acquired
in a transaction.

v1: original
v2: remove a few things not needed until atomic, for now
v3: update for v3 of connection_mutex patch..
v4: squash in docbook
v5: doc tweaks/fixes

Signed-off-by: Rob Clark <robdclark@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
2014-06-05 09:54:33 +10:00
Dave Airlie 885ae1c55a Merge tag 'topic/core-stuff-2014-06-02' of git://anongit.freedesktop.org/drm-intel into drm-next
Just flushing out my pile of random drm patches for the merge window,
nothing big. And it all hung around in drm-intel trees for a while (only
just rebased now).

* tag 'topic/core-stuff-2014-06-02' of git://anongit.freedesktop.org/drm-intel:
  imx-drm: imx-tve: remove unused variable
  drm: Missed clflushopt in drm_clflush_virt_range
  drm/plane: Fix a couple of checkpatch warnings
  drm/plane: Fix sparse warnings
  drm/exynos: Fix double locks at PM resume
  drm/ast: Fix double lock at PM resume
  drm/dp-helper: Deprecate old i2c-over-dp_aux heleprs
2014-06-04 15:47:41 +10:00
Daniel Vetter 6e9f798d91 drm: Split connection_mutex out of mode_config.mutex (v3)
After the split-out of crtc locks from the big mode_config.mutex
there's still two major areas it protects:
- Various connector probe states, like connector->status, EDID
  properties, probed mode lists and similar information.
- The links from connector->encoder and encoder->crtc and other
  modeset-relevant connector state (e.g. properties which control the
  panel fitter).

The later is used by modeset operations. But they don't really care
about the former since it's allowed to e.g. enable a disconnected VGA
output or with a mode not in the probed list.

Thus far this hasn't been a problem, but for the atomic modeset
conversion Rob Clark needs to convert all modeset relevant locks into
w/w locks. This is required because the order of acquisition is
determined by how userspace supplies the atomic modeset data. This has
run into troubles in the detect path since the i915 load detect code
needs _both_ protections offered by the mode_config.mutex: It updates
probe state and it needs to change the modeset configuration to enable
the temporary load detect pipe.

The big deal here is that for the probe/detect users of this lock a
plain mutex fits best, but for atomic modesets we really want a w/w
mutex. To fix this lets split out a new connection_mutex lock for the
modeset relevant parts.

For simplicity I've decided to only add one additional lock for all
connector/encoder links and modeset configuration states. We have
piles of different modeset objects in addition to those (like bridges
or panels), so adding per-object locks would be much more effort.

Also, we're guaranteed (at least for now) to do a full modeset if we
need to acquire this lock. Which means that fine-grained locking is
fairly irrelevant compared to the amount of time the full modeset will
take.

I've done a full audit, and there's just a few things that justify
special focus:
- Locking in drm_sysfs.c is almost completely absent. We should
  sprinkle mode_config.connection_mutex over this file a bit, but
  since it already lacks mode_config.mutex this patch wont make the
  situation any worse. This is material for a follow-up patch.

- omap has a omap_framebuffer_flush function which walks the
  connector->encoder->crtc links and is called from many contexts.
  Some look like they don't acquire mode_config.mutex, so this is
  already racy. Again fixing this is material for a separate patch.

- The radeon hot_plug function to retrain DP links looks at
  connector->dpms. Currently this happens without any locking, so is
  already racy. I think radeon_hotplug_work_func should gain
  mutex_lock/unlock calls for the mode_config.connection_mutex.

- Same applies to i915's intel_dp_hot_plug. But again, this is already
  racy.

- i915 load_detect code needs to acquire this lock. Which means the
  w/w dance due to Rob's work will be nicely contained to _just_ this
  function.

I've added fixme comments everywhere where it looks suspicious but in
the sysfs code. After a quick irc discussion with Dave Airlie it
sounds like the lack of locking in there is due to sysfs cleanup fun
at module unload.

v1: original (only compile tested)

v2: missing mutex_init(), etc (from Rob Clark)

v3: i915 needs more care in the conversion:
- Protect the edp pp logic with the connection_mutex.
- Use connection_mutex in the backlight code due to
  get_pipe_from_connector.
- Use drm_modeset_lock_all in suspend/resume paths.
- Update lock checks in the overlay code.

Cc: Alex Deucher <alexdeucher@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Rob Clark <robdclark@gmail.com>
2014-06-04 13:25:21 +10:00
Thierry Reding 233fd4ec92 drm/plane: Fix a couple of checkpatch warnings
Code should be indented using tabs rather than spaces (see CodingStyle)
and the canonical form to declare a constant static variable is using
"static const" rather than "const static". Fixes the following warnings
from checkpatch:

	$ scripts/checkpatch.pl -f drivers/gpu/drm/drm_plane_helper.c
	WARNING: storage class should be at the beginning of the declaration
	#40: FILE: drivers/gpu/drm/drm_plane_helper.c:40:
	+const static uint32_t safe_modeset_formats[] = {

	WARNING: please, no spaces at the start of a line
	#41: FILE: drivers/gpu/drm/drm_plane_helper.c:41:
	+       DRM_FORMAT_XRGB8888,$

	WARNING: please, no spaces at the start of a line
	#42: FILE: drivers/gpu/drm/drm_plane_helper.c:42:
	+       DRM_FORMAT_ARGB8888,$

Signed-off-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-06-02 09:57:31 +02:00
Thierry Reding f220e62659 drm/plane: Fix sparse warnings
Include the drm_plane_helper.h header file to fix the following sparse
warnings:

	  CHECK   drivers/gpu/drm/drm_plane_helper.c
	drivers/gpu/drm/drm_plane_helper.c:102:5: warning: symbol 'drm_primary_helper_update' was not declared. Should it be static?
	drivers/gpu/drm/drm_plane_helper.c:219:5: warning: symbol 'drm_primary_helper_disable' was not declared. Should it be static?
	drivers/gpu/drm/drm_plane_helper.c:233:6: warning: symbol 'drm_primary_helper_destroy' was not declared. Should it be static?
	drivers/gpu/drm/drm_plane_helper.c:241:30: warning: symbol 'drm_primary_helper_funcs' was not declared. Should it be static?
	drivers/gpu/drm/drm_plane_helper.c:259:18: warning: symbol 'drm_primary_helper_create_plane' was not declared. Should it be static?

Doing that makes gcc complain as follows:

	  CC      drivers/gpu/drm/drm_plane_helper.o
	drivers/gpu/drm/drm_plane_helper.c:260:19: error: conflicting types for 'drm_primary_helper_create_plane'
	 struct drm_plane *drm_primary_helper_create_plane(struct drm_device *dev,
	                   ^
	In file included from drivers/gpu/drm/drm_plane_helper.c:29:0:
	include/drm/drm_plane_helper.h:42:19: note: previous declaration of 'drm_primary_helper_create_plane' was here
	 struct drm_plane *drm_primary_helper_create_plane(struct drm_device *dev,
	                   ^
	drivers/gpu/drm/drm_plane_helper.c: In function 'drm_primary_helper_create_plane':
	drivers/gpu/drm/drm_plane_helper.c:274:11: warning: assignment discards 'const' qualifier from pointer target type
	   formats = safe_modeset_formats;
	           ^
	In file included from include/linux/linkage.h:6:0,
	                 from include/linux/kernel.h:6,
	                 from include/drm/drmP.h:45,
	                 from drivers/gpu/drm/drm_plane_helper.c:27:
	drivers/gpu/drm/drm_plane_helper.c: At top level:
	drivers/gpu/drm/drm_plane_helper.c:289:15: error: conflicting types for 'drm_primary_helper_create_plane'
	 EXPORT_SYMBOL(drm_primary_helper_create_plane);
	               ^
	include/linux/export.h:57:21: note: in definition of macro '__EXPORT_SYMBOL'
	  extern typeof(sym) sym;     \
	                     ^
	drivers/gpu/drm/drm_plane_helper.c:289:1: note: in expansion of macro 'EXPORT_SYMBOL'
	 EXPORT_SYMBOL(drm_primary_helper_create_plane);
	 ^
	In file included from drivers/gpu/drm/drm_plane_helper.c:29:0:
	include/drm/drm_plane_helper.h:42:19: note: previous declaration of 'drm_primary_helper_create_plane' was here
	 struct drm_plane *drm_primary_helper_create_plane(struct drm_device *dev,
	                   ^

Which can easily be fixed by making the signatures of the implementation
and the prototype match.

Signed-off-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-06-02 09:57:30 +02:00
Daniel Vetter 0fe27f063f drm: Simplify fb refcounting rules around ->update_plane
The introduction of primary planes has apparently caused a bit of fb
refcounting fun for people. That makes it a good time to clean up the
arcane rules and slight differences between ->update_plane and
->set_config. The new rules are:

- The core holds a reference for both the new and the old fb (if
  they're non-NULL of course) while calling into the driver through
  either ->update_plane or ->set_config.

- Drivers may not clobber plane->fb if their callback fails. If they
  do that, they need to store a pointer to the old fb in it again.
  When calling into the driver plane->fb still points at the current
  (old) framebuffer.

- The core will update the plane->fb pointer on success. Drivers can
  do that themselves too, but aren't required to any more for the
  primary plane.

- The core will update fb refcounts for the plane->fb pointer,
  presuming the drivers hold up their end of the bargain.

v2: Remove now unused tmpfb (Thierry)

v3: Drop broken changes from drm_mode_setplane (Ville). Also polish
the commit message a bit.

v4: Also fix up the handling of ->disable_plane in
drm_plane_force_disable. The issue was that we didn't save plane->fb
over the ->disable_plane call. Just paranoia, nothing relies on this.

v5: Keep still useful comments about directly calling ->set_config,
which I should have done for v4 already. Requested by Matt.

Cc: Thierry Reding <treding@nvidia.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-04-23 20:06:47 +02:00
Daniel Vetter 3c858a3385 drm/plane_helper: don't disable plane in destroy function
By the time drm_mode_config_cleanup calls this all the hw state should
be cleaned up already - we even have a WARN right before calling
plane->destroy callbacks asserting that all framebuffers are gone.

So trying to disable things harder is a bit a bug. Caught by Thierry
since it resulted in some mode_config.mutex locking backtraces.

Cc: Thierry Reding <treding@nvidia.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Tested-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-04-22 11:19:09 +02:00
Matt Roper c10dddc1d5 drm/plane-helper: Fix primary plane scaling check
The src_w / src_h parameters to update_plane include a subpixel offset;
we need to shift off the subpixel bits before comparing to CRTC size
when checking for primary plane scaling.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
2014-04-22 11:19:08 +02:00
Daniel Vetter b6ccd7b987 drm/plane-helper: Don't fake-implement primary plane disabling
After thinking about this topic a bit more I've reached the conclusion
that implementing this doesn't make sense:

- The locking is all wrong: set_config(NULL) will also unlink encoders
  and connectors, but those links are protected with the mode_config
  mutex. In the ->disable_plane callback we only hold all modeset
  locks, but eventually we want to switch to just grabbing the
  per-crtc (and maybe per-plane) locks as needed, maybe based on
  ww_mutexes. Having a callback which absolutely needs all modeset
  locks is bad for this conversion.

  Note that the same isn't true for the provided ->update_plane since
  we've audited the crtc helpers to make sure that not encoder or
  connector links are changed.

- There's no way to re-enable the plane with an ->update_plane: The
  connectors/encoder links are lost and so we can't re-enable the
  CRTC. Even without that issue the driver might have reassigned some
  shared resources (as opposed to e.g. DPMS off, where drivers are not
  allowed to do that to make sure the CRTC can be enabled again).

- The semantics don't make much sense: Userspace asked to scan out
  black (or some other color if the driver supports a background
  color), not that the screen be disabled.

- Implementing proper primary plane support (i.e. actually disabling
  the primary plane without disabling the CRTC) is really simple, at
  least if all the hw needs is flipping a bit. The big task is
  auditing all the interactions with other ioctls when the CRTC is on
  but there's no primary plane (e.g. pageflips). And some of that work
  still needs to be done.

Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
2014-04-18 13:18:50 +10:00
Matt Roper e13161af80 drm: Add drm_crtc_init_with_planes() (v2)
Add a new drm_crtc_init_with_planes() to allow drivers to provide
specific primary and cursor planes at CRTC initialization.  The existing
drm_crtc_init() interface remains to avoid driver churn in existing
drivers; it will initialize the CRTC with a plane helper-created primary
plane and no cursor plane.

v2:
  - Move drm_crtc_init() to plane helper file so that nothing in the DRM
    core depends on helpers.  [suggested by Daniel Vetter]
  - Keep cursor parameter to drm_crtc_init_with_planes() a void* until
    we actually add cursor support.  [suggested by Daniel Vetter]

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
2014-04-01 20:18:27 -04:00
Matt Roper c103d1cfb3 drm: Add primary plane helpers (v3)
When we expose non-overlay planes to userspace, they will become
accessible via standard userspace plane API's.  We should be able to
handle the standard plane operations against primary planes in a generic
way via the modeset handler.

Drivers that can program primary planes more efficiently, that want to
use their own primary plane structure to track additional information,
or that don't have the limitations assumed by the helpers are free to
provide their own implementation of some or all of these handlers.

v3: Tweak kerneldoc formatting slightly to avoid ugliness
v2:
 - Move plane helpers to a new file (drm_plane_helper.c)
 - Tighten checks on update handler (check for scaling, CRTC coverage,
   subpixel positioning)
 - Pass proper panning parameters to modeset interface
 - Disallow disabling primary plane (and thus CRTC) if other planes are
   still active on the CRTC.
 - Use a minimal format list that should work on all hardware/drivers.
   Drivers may call this function with a more accurate plane list to
   enable additional formats they can support.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
2014-04-01 20:11:28 -04:00