From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Chen Feng <puck.chen@hisilicon.com>,
lkml <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Xinliang Liu <z.liuxinliang@hisilicon.com>,
Xinwei Kong <kong.kongxinwei@hisilicon.com>,
Rongrong Zou <zourongrong@gmail.com>,
Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
Date: Tue, 14 Feb 2017 21:51:29 +0200 [thread overview]
Message-ID: <20170214195129.GN31595@intel.com> (raw)
In-Reply-To: <CAKMK7uED5JQG3Kv1O1zdhrpQ=0S5+HSZVeb1enzeaG7JnVi_8g@mail.gmail.com>
On Tue, Feb 14, 2017 at 08:38:40PM +0100, Daniel Vetter wrote:
> On Tue, Feb 14, 2017 at 8:25 PM, John Stultz <john.stultz@linaro.org> wrote:
> > Currently, on the hikey board, we have the adv7511 bridge wired
> > up to the kirin ade drm driver. Unfortunately, the kirin ade
> > core cannot generate accurate byteclocks for all pixel clock
> > values.
> >
> > Thus if a mode clock is selected that we cannot calculate a
> > matching byteclock, the device will boot with a blank screen.
> >
> > Unfortunately, currently the only place we can properly check
> > potential modes for this issue in the connector mode_valid
> > helper. Again, hikey uses the adv7511 bridge, which is shared
> > between a number of different devices, so its improper to put
> > restrictions caused by the kirin drm driver in the adv7511
> > logic.
> >
> > So this patch tries to correct for that, by adding some
> > infrastructure so that the drm_crtc_helper_funcs can optionally
> > implement a mode_valid check, so that the probe helpers can
> > check to make sure there are not any restrictions at the crtc
> > level as well.
> >
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Xinliang Liu <xinliang.liu@linaro.org>
> > Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
> > Cc: Rongrong Zou <zourongrong@gmail.com>
> > Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> > Cc: Chen Feng <puck.chen@hisilicon.com>
> > Cc: Archit Taneja <architt@codeaurora.org>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
>
> So I'm going to be super-annoying here and ask for a complete
> solution. This here is defacto what ever driver already does (or has
> too), but it doesn't really solve the overall issue of having entirely
> separate validation paths for probe and atomic_check paths. I think if
> we wan to solve this, we need to solve this properly, with a generic
> solution. That would mean:
> - still in helpers, to make it all opt-int
> - covers crtc and encoders and bridges
> - allows you to implement the current mode_valid in terms of the new
> stuff (maybe as a default hook)
> - allows you to implement the current assortment of mode_fixup and/or
> atomic_check in terms of the new stuff, or at least to not have to
> duplicate logic in there
Long ago I quickly looked at doing this for i915. IIRC the main
complication was the normal vs. the crtc_ timings stored in the
mode. I suppose one option would be to populate the crtc_ timings
in .mode_valid() as well and otherwise just ignore the normal timings.
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: John Stultz <john.stultz@linaro.org>,
Chen Feng <puck.chen@hisilicon.com>,
lkml <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Xinliang Liu <z.liuxinliang@hisilicon.com>,
Xinwei Kong <kong.kongxinwei@hisilicon.com>,
Rongrong Zou <zourongrong@gmail.com>,
Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
Date: Tue, 14 Feb 2017 21:51:29 +0200 [thread overview]
Message-ID: <20170214195129.GN31595@intel.com> (raw)
In-Reply-To: <CAKMK7uED5JQG3Kv1O1zdhrpQ=0S5+HSZVeb1enzeaG7JnVi_8g@mail.gmail.com>
On Tue, Feb 14, 2017 at 08:38:40PM +0100, Daniel Vetter wrote:
> On Tue, Feb 14, 2017 at 8:25 PM, John Stultz <john.stultz@linaro.org> wrote:
> > Currently, on the hikey board, we have the adv7511 bridge wired
> > up to the kirin ade drm driver. Unfortunately, the kirin ade
> > core cannot generate accurate byteclocks for all pixel clock
> > values.
> >
> > Thus if a mode clock is selected that we cannot calculate a
> > matching byteclock, the device will boot with a blank screen.
> >
> > Unfortunately, currently the only place we can properly check
> > potential modes for this issue in the connector mode_valid
> > helper. Again, hikey uses the adv7511 bridge, which is shared
> > between a number of different devices, so its improper to put
> > restrictions caused by the kirin drm driver in the adv7511
> > logic.
> >
> > So this patch tries to correct for that, by adding some
> > infrastructure so that the drm_crtc_helper_funcs can optionally
> > implement a mode_valid check, so that the probe helpers can
> > check to make sure there are not any restrictions at the crtc
> > level as well.
> >
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Xinliang Liu <xinliang.liu@linaro.org>
> > Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
> > Cc: Rongrong Zou <zourongrong@gmail.com>
> > Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> > Cc: Chen Feng <puck.chen@hisilicon.com>
> > Cc: Archit Taneja <architt@codeaurora.org>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
>
> So I'm going to be super-annoying here and ask for a complete
> solution. This here is defacto what ever driver already does (or has
> too), but it doesn't really solve the overall issue of having entirely
> separate validation paths for probe and atomic_check paths. I think if
> we wan to solve this, we need to solve this properly, with a generic
> solution. That would mean:
> - still in helpers, to make it all opt-int
> - covers crtc and encoders and bridges
> - allows you to implement the current mode_valid in terms of the new
> stuff (maybe as a default hook)
> - allows you to implement the current assortment of mode_fixup and/or
> atomic_check in terms of the new stuff, or at least to not have to
> duplicate logic in there
Long ago I quickly looked at doing this for i915. IIRC the main
complication was the normal vs. the crtc_ timings stored in the
mode. I suppose one option would be to populate the crtc_ timings
in .mode_valid() as well and otherwise just ignore the normal timings.
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2017-02-14 19:51 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-14 19:25 [RFC][PATCH 0/2] Add mode_valid drm_crtc_helper_funcs for HiKey John Stultz
2017-02-14 19:25 ` John Stultz
2017-02-14 19:25 ` [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs John Stultz
2017-02-14 19:25 ` John Stultz
2017-02-14 19:38 ` Daniel Vetter
2017-02-14 19:38 ` Daniel Vetter
2017-02-14 19:45 ` John Stultz
2017-02-14 19:45 ` John Stultz
2017-02-14 20:22 ` Daniel Vetter
2017-02-14 20:22 ` Daniel Vetter
2017-02-14 21:03 ` John Stultz
2017-02-14 21:03 ` John Stultz
2017-02-14 21:42 ` Daniel Vetter
2017-02-14 21:42 ` Daniel Vetter
2017-02-15 16:54 ` Maxime Ripard
2017-02-15 16:54 ` Maxime Ripard
2017-02-14 19:51 ` Ville Syrjälä [this message]
2017-02-14 19:51 ` Ville Syrjälä
2017-02-14 20:32 ` Daniel Stone
2017-02-14 20:32 ` Daniel Stone
2017-02-14 21:07 ` John Stultz
2017-02-14 21:07 ` John Stultz
2017-02-14 21:49 ` Daniel Vetter
2017-02-14 21:49 ` Daniel Vetter
2017-02-15 2:21 ` Rob Clark
2017-02-15 2:21 ` Rob Clark
2017-02-28 8:08 ` Mark yao
2017-02-20 22:32 ` Daniel Vetter
2017-02-20 22:32 ` Daniel Vetter
2017-02-28 6:03 ` John Stultz
2017-02-28 6:03 ` John Stultz
2017-02-28 9:34 ` Daniel Vetter
2017-02-28 9:34 ` Daniel Vetter
2017-02-14 19:25 ` [RFC][PATCH 2/2] drm: kirin: Restrict modes to known good mode clocks John Stultz
2017-02-14 19:25 ` John Stultz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170214195129.GN31595@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=daniel.vetter@intel.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=kong.kongxinwei@hisilicon.com \
--cc=linux-kernel@vger.kernel.org \
--cc=puck.chen@hisilicon.com \
--cc=z.liuxinliang@hisilicon.com \
--cc=zourongrong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.