16 Aug, 2016

1 commit

  • While reviewing docs I spotted that we have a few functions that
    really just don't fit into their containing helper library section.
    Extract them and shovel them all into a new library for random one-off
    aux stuff.

    v2: Remove wrongly added files for real.

    Cc: Sean Paul
    Reviewed-by: Sean Paul
    Signed-off-by: Daniel Vetter
    Link: http://patchwork.freedesktop.org/patch/msgid/1471034937-651-3-git-send-email-daniel.vetter@ffwll.ch

    Daniel Vetter
     

09 Aug, 2016

1 commit

  • Add a version of drm_plane_helper_check_update() which takes a plane
    state instead of having the caller pass in everything.

    And to reduce code duplication, let's reimplement
    drm_plane_helper_check_update() in terms of the new function, by
    having a tempororary plane state on the stack.

    v2: Add a note that the functions modifies the state (Chris)
    v3: Fix drm_plane_helper_check_update() y coordinates (Daniel Kurtz)

    Cc: Daniel Kurtz
    Cc: Chris Wilson
    Signed-off-by: Ville Syrjälä
    Reviewed-by: Sean Paul (v2)
    Signed-off-by: Ville Syrjälä
    Signed-off-by: Sean Paul
    Link: http://patchwork.freedesktop.org/patch/msgid/1470642910-14073-1-git-send-email-ville.syrjala@linux.intel.com

    Ville Syrjälä
     

17 Jun, 2016

1 commit

  • drm_plane_helper_check_update() needs to account for the plane rotation
    for correct clipping/scaling calculations. Do so.

    There was an earlier attempt [1] to add this into
    intel_check_primary_plane() but I requested that it'd be put into the
    helper instead. An updated patch never materialized AFAICS, so I went
    ahead and cooked one up myself.

    v2: Deal with new drm_plane_helper_check_update() callers

    [1] https://patchwork.freedesktop.org/patch/65177/
    Cc: Nabendu Maiti
    Cc: Noralf Trønnes
    Cc: CK Hu
    Cc: Mark Yao
    Cc: Russell King
    Signed-off-by: Ville Syrjälä
    Reviewed-by: Patrik Jakobsson
    Signed-off-by: Daniel Vetter
    Link: http://patchwork.freedesktop.org/patch/msgid/1466172790-10025-1-git-send-email-ville.syrjala@linux.intel.com

    Ville Syrjälä
     

08 Dec, 2015

1 commit

  • Currently we have 4 helper libraries (probe, crtc, plane & atomic)
    that all use the same helper vtables. And that's by necessity since we
    don't want to litter the core structs with one ops pointer per helper
    library. Also often the reuse the same hooks (like atomic does, to
    facilite conversion from existing drivers using crtc and plane
    helpers).

    Given all that it doesn't make sense to put the docs for these next to
    specific helpers. Instead extract them into a new header file and
    section in the docbook, and add references to them everywhere.

    Unfortunately kernel-doc complains when an include directive doesn't
    find anything (and it does by dumping crap into the output file). We
    have to remove the now empty includes to avoid that, instead of leaving
    them in for future proofing.

    v2: More OCD in ordering functions.

    v3: Spelling plus collate copyright headers properly.

    Signed-off-by: Daniel Vetter
    Link: http://patchwork.freedesktop.org/patch/msgid/1449218769-16577-4-git-send-email-daniel.vetter@ffwll.ch
    Reviewed-by: Thierry Reding

    Daniel Vetter
     

08 Sep, 2015

1 commit

  • This removes the need to separately track fb changes i915.
    That will be done as a separate commit, however.

    Changes since v1:
    - Add dri-devel to cc.
    - Fix a check in intel's prepare and cleanup fb to take rotation
    into account.
    Changes since v2:
    - Split out i915 changes to a separate commit.

    Cc: dri-devel@lists.freedesktop.org
    Signed-off-by: Maarten Lankhorst
    Reviewed-by: Daniel Stone
    [danvet: Squash in msm fixup from Maarten.]
    Signed-off-by: Daniel Vetter

    Maarten Lankhorst
     

12 Aug, 2015

1 commit


13 Apr, 2015

1 commit


10 Mar, 2015

1 commit


05 Mar, 2015

1 commit

  • 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
    Cc: Daniel Vetter
    Cc: dri-devel@lists.freedesktop.org
    Reviewed-by: Rob Clark
    Signed-off-by: Dave Airlie

    Tvrtko Ursulin
     

27 Jan, 2015

2 commits

  • 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
    Reviewed-by: Gustavo Padovan
    Signed-off-by: Thierry Reding

    Thierry Reding
     
  • There is no use-case where it would be useful for drivers not to
    implement this function and the transitional plane helpers already
    require drivers to provide an implementation.

    v2: add new requirement to kerneldoc

    Reviewed-by: Daniel Vetter
    Signed-off-by: Thierry Reding

    Thierry Reding
     

25 Nov, 2014

2 commits

  • The plane helpers aren't pulled into the DocBook yet, so these weren't
    noticed.

    Reviewed-by: Daniel Vetter
    Signed-off-by: Thierry Reding
    Signed-off-by: Daniel Vetter

    Thierry Reding
     
  • 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
    Signed-off-by: Thierry Reding
    Signed-off-by: Daniel Vetter

    Thierry Reding
     

06 Nov, 2014

3 commits

  • 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
    Signed-off-by: Daniel Vetter

    Daniel Vetter
     
  • 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
    Cc: Sean Paul
    Reviewed-by: Sean Paul
    Signed-off-by: Daniel Vetter

    Daniel Vetter
     
  • This is the first cut of atomic helper code. As-is it's only useful to
    implement a pure atomic interface for plane updates.

    Later patches will integrate this with the crtc helpers so that full
    atomic updates are possible. We also need a pile of helpers to aid
    drivers in transitioning from the legacy world to the shiny new atomic
    age. Finally we need helpers to implement legacy ioctls on top of the
    atomic interface.

    The design of the overall helpersdriver interaction is fairly
    simple, but has an unfortunate large interface:

    - We have ->atomic_check callbacks for crtcs and planes. The idea is
    that connectors don't need any checking, and if they do they can
    adjust the relevant crtc driver-private state. So no connector hooks
    should be needed. Also the crtc helpers integration will do the
    ->best_encoder checks, so no need for that.

    - Framebuffer pinning needs to be done before we can commit to the hw
    state. This is especially important for async updates where we must
    pin all buffers before returning to userspace, so that really only
    hw failures can happen in the asynchronous worker.

    Hence we add ->prepare_fb and ->cleanup_fb hooks for this resources
    management.

    - The actual atomic plane commit can't fail (except hw woes), so has
    void return type. It has three stages:
    1. Prepare all affected crtcs with crtc->atomic_begin. Drivers can
    use this to unset the GO bit or similar latches to prevent plane
    updates.
    2. Update plane state by looping over all changed planes and calling
    plane->atomic_update. Presuming the hardware is sane and has GO
    bits drivers can simply bash the state into the hardware in this
    function. Other drivers might use this to precompute hw state for
    the final step.
    3. Finally latch the update for the next vblank with
    crtc->atomic_flush. Note that this function doesn't need to wait
    for the vblank to happen even for the synchronous case.

    v2: Clear drm__state->state to NULL when swapping in state.

    v3: Add TODO that we don't short-circuit plane updates for now. Likely
    no one will care.

    v4: Squash in a bit of polish that somehow landed in the wrong (later)
    patche.

    v5: Integrate atomic functions into the drm docbook and fixup the
    kerneldoc.

    v6: Fixup fixup patch squashing fumble.

    v7: Don't touch the legacy plane state plane->fb and plane->crtc. This
    is only used by the legacy ioctl code in the drm core, and that code
    already takes care of updating the pointers in all relevant cases.
    This is in stark contrast to connector->encoder->crtc links on the
    modeset side, which we still need to set since the core doesn't touch
    them.

    Also some more kerneldoc polish.

    v8: Drop outdated comment.

    v9: Handle the state->state pointer correctly: Only clearing the
    ->state pointer when assigning the state to the kms object isn't good
    enough. We also need to re-link the swapped out state into the
    drm_atomic_state structure.

    v10: Shuffle the misplaced docbook template hunk around that Sean spotted.

    Cc: Sean Paul
    Reviewed-by: Sean Paul
    Signed-off-by: Daniel Vetter

    Daniel Vetter
     

05 Nov, 2014

1 commit

  • Just a bit of OCD cleanup on headers - this function isn't the core
    interface any more but just a helper for drivers who haven't yet
    transitioned to universal planes. Put the declaration at the right
    spot and sprinkle necessary #includes over all drivers.

    Maybe this helps to encourage driver maintainers to do the switch.

    v2: Fix #include ordering for tegra, reported by 0-day builder.

    v3: Include required headers, reported by Thierry.

    Cc: Matt Roper
    Cc: Thierry Reding
    Reviewed-by: Matt Roper
    Reviewed-by: Sean Paul
    Signed-off-by: Daniel Vetter

    Daniel Vetter
     

05 Jun, 2014

1 commit

  • 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
    Signed-off-by: Matt Roper
    Acked-by: Dave Airlie
    [danvet: Include header properly and fixup declaration mismatch to
    make this compile.]
    Signed-off-by: Daniel Vetter

    Matt Roper
     

02 Jun, 2014

1 commit

  • 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
    Reviewed-by: Matt Roper
    Signed-off-by: Daniel Vetter

    Thierry Reding
     

02 Apr, 2014

1 commit

  • 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
    Reviewed-by: Rob Clark

    Matt Roper