* [PATCH] drm/i915: Skip DDI PLL selection for DSI @ 2016-02-05 11:29 Mika Kahola 2016-02-05 11:47 ` ✗ Fi.CI.BAT: failure for " Patchwork 2016-02-09 2:52 ` [PATCH] " Thulasimani, Sivakumar 0 siblings, 2 replies; 7+ messages in thread From: Mika Kahola @ 2016-02-05 11:29 UTC (permalink / raw) To: intel-gfx; +Cc: jani.nikula Skip DDI PLL selection if display type is DSI/MIPI. Signed-off-by: Mika Kahola <mika.kahola@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d7de2a5..5da98b2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9902,8 +9902,13 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state) static int haswell_crtc_compute_clock(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state) { - if (!intel_ddi_pll_select(crtc, crtc_state)) - return -EINVAL; + struct intel_encoder *intel_encoder = + intel_ddi_get_crtc_new_encoder(crtc_state); + + if (intel_encoder->type != INTEL_OUTPUT_DSI) { + if (!intel_ddi_pll_select(crtc, crtc_state)) + return -EINVAL; + } crtc->lowfreq_avail = false; -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Skip DDI PLL selection for DSI 2016-02-05 11:29 [PATCH] drm/i915: Skip DDI PLL selection for DSI Mika Kahola @ 2016-02-05 11:47 ` Patchwork 2016-02-09 2:52 ` [PATCH] " Thulasimani, Sivakumar 1 sibling, 0 replies; 7+ messages in thread From: Patchwork @ 2016-02-05 11:47 UTC (permalink / raw) To: Mika Kahola; +Cc: intel-gfx == Summary == Series 3122v1 drm/i915: Skip DDI PLL selection for DSI http://patchwork.freedesktop.org/api/1.0/series/3122/revisions/1/mbox/ Test gem_sync: Subgroup basic-blt: pass -> INCOMPLETE (skl-i5k-2) Test kms_flip: Subgroup basic-flip-vs-dpms: dmesg-warn -> PASS (ilk-hp8440p) UNSTABLE bdw-nuci7 total:161 pass:152 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultra total:164 pass:152 dwarn:0 dfail:0 fail:0 skip:12 byt-nuc total:164 pass:141 dwarn:0 dfail:0 fail:0 skip:23 hsw-brixbox total:164 pass:151 dwarn:0 dfail:0 fail:0 skip:13 hsw-gt2 total:164 pass:154 dwarn:0 dfail:0 fail:0 skip:10 ilk-hp8440p total:164 pass:116 dwarn:0 dfail:0 fail:0 skip:48 ivb-t430s total:164 pass:150 dwarn:0 dfail:0 fail:0 skip:14 skl-i5k-2 total:57 pass:48 dwarn:0 dfail:0 fail:0 skip:8 skl-i7k-2 total:164 pass:149 dwarn:1 dfail:0 fail:0 skip:14 snb-dellxps total:164 pass:142 dwarn:0 dfail:0 fail:0 skip:22 Results at /archive/results/CI_IGT_test/Patchwork_1369/ 57e229193395068adcb34c5266d54194e652869f drm-intel-nightly: 2016y-02m-04d-18h-38m-55s UTC integration manifest aa42a6ab884b76ece511db816e90b80b6bd3a7ef drm/i915: Skip DDI PLL selection for DSI _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Skip DDI PLL selection for DSI 2016-02-05 11:29 [PATCH] drm/i915: Skip DDI PLL selection for DSI Mika Kahola 2016-02-05 11:47 ` ✗ Fi.CI.BAT: failure for " Patchwork @ 2016-02-09 2:52 ` Thulasimani, Sivakumar 2016-02-09 6:32 ` Jani Nikula 1 sibling, 1 reply; 7+ messages in thread From: Thulasimani, Sivakumar @ 2016-02-09 2:52 UTC (permalink / raw) To: Mika Kahola, intel-gfx; +Cc: jani.nikula On 2/5/2016 4:59 PM, Mika Kahola wrote: > Skip DDI PLL selection if display type is DSI/MIPI. > > Signed-off-by: Mika Kahola <mika.kahola@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index d7de2a5..5da98b2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9902,8 +9902,13 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state) > static int haswell_crtc_compute_clock(struct intel_crtc *crtc, > struct intel_crtc_state *crtc_state) > { > - if (!intel_ddi_pll_select(crtc, crtc_state)) > - return -EINVAL; > + struct intel_encoder *intel_encoder = > + intel_ddi_get_crtc_new_encoder(crtc_state); > + > + if (intel_encoder->type != INTEL_OUTPUT_DSI) { > + if (!intel_ddi_pll_select(crtc, crtc_state)) > + return -EINVAL; > + } > can this be moved inside bxt_ddi_pll_select ? we can avoid this check for other platforms that also execute this function. regards, Sivakumar > crtc->lowfreq_avail = false; > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Skip DDI PLL selection for DSI 2016-02-09 2:52 ` [PATCH] " Thulasimani, Sivakumar @ 2016-02-09 6:32 ` Jani Nikula 2016-02-09 7:46 ` Thulasimani, Sivakumar 0 siblings, 1 reply; 7+ messages in thread From: Jani Nikula @ 2016-02-09 6:32 UTC (permalink / raw) To: Thulasimani, Sivakumar, Mika Kahola, intel-gfx On Tue, 09 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote: > On 2/5/2016 4:59 PM, Mika Kahola wrote: >> Skip DDI PLL selection if display type is DSI/MIPI. >> >> Signed-off-by: Mika Kahola <mika.kahola@intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index d7de2a5..5da98b2 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -9902,8 +9902,13 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state) >> static int haswell_crtc_compute_clock(struct intel_crtc *crtc, >> struct intel_crtc_state *crtc_state) >> { >> - if (!intel_ddi_pll_select(crtc, crtc_state)) >> - return -EINVAL; >> + struct intel_encoder *intel_encoder = >> + intel_ddi_get_crtc_new_encoder(crtc_state); >> + >> + if (intel_encoder->type != INTEL_OUTPUT_DSI) { >> + if (!intel_ddi_pll_select(crtc, crtc_state)) >> + return -EINVAL; >> + } >> > can this be moved inside bxt_ddi_pll_select ? we can avoid this check for > other platforms that also execute this function. I asked Mika to do it this way, but if you feel strongly about it I guess I could be persuaded otherwise too. My main point is, if we pass on DSI encoders to DDI functions in some cases but mostly not, it will muddy the waters and eventually people end up checking for "is dsi" all around DDI just because they can't be bothered to check if the functions are really called for DDI only or not. It's more of a maintainability concern than anything else. BR, Jani. > > regards, > Sivakumar >> crtc->lowfreq_avail = false; >> > -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Skip DDI PLL selection for DSI 2016-02-09 6:32 ` Jani Nikula @ 2016-02-09 7:46 ` Thulasimani, Sivakumar 2016-02-09 9:52 ` Ville Syrjälä 0 siblings, 1 reply; 7+ messages in thread From: Thulasimani, Sivakumar @ 2016-02-09 7:46 UTC (permalink / raw) To: Jani Nikula, Mika Kahola, intel-gfx On 2/9/2016 12:02 PM, Jani Nikula wrote: > On Tue, 09 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote: >> On 2/5/2016 4:59 PM, Mika Kahola wrote: >>> Skip DDI PLL selection if display type is DSI/MIPI. >>> >>> Signed-off-by: Mika Kahola <mika.kahola@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_display.c | 9 +++++++-- >>> 1 file changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >>> index d7de2a5..5da98b2 100644 >>> --- a/drivers/gpu/drm/i915/intel_display.c >>> +++ b/drivers/gpu/drm/i915/intel_display.c >>> @@ -9902,8 +9902,13 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state) >>> static int haswell_crtc_compute_clock(struct intel_crtc *crtc, >>> struct intel_crtc_state *crtc_state) >>> { >>> - if (!intel_ddi_pll_select(crtc, crtc_state)) >>> - return -EINVAL; >>> + struct intel_encoder *intel_encoder = >>> + intel_ddi_get_crtc_new_encoder(crtc_state); >>> + >>> + if (intel_encoder->type != INTEL_OUTPUT_DSI) { >>> + if (!intel_ddi_pll_select(crtc, crtc_state)) >>> + return -EINVAL; >>> + } >>> >> can this be moved inside bxt_ddi_pll_select ? we can avoid this check for >> other platforms that also execute this function. > I asked Mika to do it this way, but if you feel strongly about it I > guess I could be persuaded otherwise too. > > My main point is, if we pass on DSI encoders to DDI functions in some > cases but mostly not, it will muddy the waters and eventually people end > up checking for "is dsi" all around DDI just because they can't be > bothered to check if the functions are really called for DDI only or > not. It's more of a maintainability concern than anything else. > > BR, > Jani. > i am fine with this either way. i was thinking of avoid such checks in other platforms where it is not needed but your concern of too many is_dsi checks is valid as well. with that i am fine with this change as is. Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > >> regards, >> Sivakumar >>> crtc->lowfreq_avail = false; >>> _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Skip DDI PLL selection for DSI 2016-02-09 7:46 ` Thulasimani, Sivakumar @ 2016-02-09 9:52 ` Ville Syrjälä 2016-02-09 15:43 ` Jani Nikula 0 siblings, 1 reply; 7+ messages in thread From: Ville Syrjälä @ 2016-02-09 9:52 UTC (permalink / raw) To: Thulasimani, Sivakumar; +Cc: Jani Nikula, intel-gfx On Tue, Feb 09, 2016 at 01:16:18PM +0530, Thulasimani, Sivakumar wrote: > > > On 2/9/2016 12:02 PM, Jani Nikula wrote: > > On Tue, 09 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote: > >> On 2/5/2016 4:59 PM, Mika Kahola wrote: > >>> Skip DDI PLL selection if display type is DSI/MIPI. > >>> > >>> Signed-off-by: Mika Kahola <mika.kahola@intel.com> > >>> --- > >>> drivers/gpu/drm/i915/intel_display.c | 9 +++++++-- > >>> 1 file changed, 7 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >>> index d7de2a5..5da98b2 100644 > >>> --- a/drivers/gpu/drm/i915/intel_display.c > >>> +++ b/drivers/gpu/drm/i915/intel_display.c > >>> @@ -9902,8 +9902,13 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state) > >>> static int haswell_crtc_compute_clock(struct intel_crtc *crtc, > >>> struct intel_crtc_state *crtc_state) > >>> { > >>> - if (!intel_ddi_pll_select(crtc, crtc_state)) > >>> - return -EINVAL; > >>> + struct intel_encoder *intel_encoder = > >>> + intel_ddi_get_crtc_new_encoder(crtc_state); > >>> + > >>> + if (intel_encoder->type != INTEL_OUTPUT_DSI) { > >>> + if (!intel_ddi_pll_select(crtc, crtc_state)) > >>> + return -EINVAL; > >>> + } > >>> > >> can this be moved inside bxt_ddi_pll_select ? we can avoid this check for > >> other platforms that also execute this function. > > I asked Mika to do it this way, but if you feel strongly about it I > > guess I could be persuaded otherwise too. > > > > My main point is, if we pass on DSI encoders to DDI functions in some > > cases but mostly not, it will muddy the waters and eventually people end > > up checking for "is dsi" all around DDI just because they can't be > > bothered to check if the functions are really called for DDI only or > > not. It's more of a maintainability concern than anything else. > > > > BR, > > Jani. > > > i am fine with this either way. i was thinking of avoid such checks > in other platforms where it is not needed but your concern of > too many is_dsi checks is valid as well. > with that i am fine with this change as is. > Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> Another idea would be to use the clock_set thing to skip it, but I think historically that has only been used to skip the PLL calculations, not the PLL selection. So might be it would just confuse things more. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Skip DDI PLL selection for DSI 2016-02-09 9:52 ` Ville Syrjälä @ 2016-02-09 15:43 ` Jani Nikula 0 siblings, 0 replies; 7+ messages in thread From: Jani Nikula @ 2016-02-09 15:43 UTC (permalink / raw) To: Ville Syrjälä, Thulasimani, Sivakumar; +Cc: intel-gfx On Tue, 09 Feb 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Tue, Feb 09, 2016 at 01:16:18PM +0530, Thulasimani, Sivakumar wrote: >> >> >> On 2/9/2016 12:02 PM, Jani Nikula wrote: >> > On Tue, 09 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote: >> >> On 2/5/2016 4:59 PM, Mika Kahola wrote: >> >>> Skip DDI PLL selection if display type is DSI/MIPI. >> >>> >> >>> Signed-off-by: Mika Kahola <mika.kahola@intel.com> >> >>> --- >> >>> drivers/gpu/drm/i915/intel_display.c | 9 +++++++-- >> >>> 1 file changed, 7 insertions(+), 2 deletions(-) >> >>> >> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> >>> index d7de2a5..5da98b2 100644 >> >>> --- a/drivers/gpu/drm/i915/intel_display.c >> >>> +++ b/drivers/gpu/drm/i915/intel_display.c >> >>> @@ -9902,8 +9902,13 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state) >> >>> static int haswell_crtc_compute_clock(struct intel_crtc *crtc, >> >>> struct intel_crtc_state *crtc_state) >> >>> { >> >>> - if (!intel_ddi_pll_select(crtc, crtc_state)) >> >>> - return -EINVAL; >> >>> + struct intel_encoder *intel_encoder = >> >>> + intel_ddi_get_crtc_new_encoder(crtc_state); >> >>> + >> >>> + if (intel_encoder->type != INTEL_OUTPUT_DSI) { >> >>> + if (!intel_ddi_pll_select(crtc, crtc_state)) >> >>> + return -EINVAL; >> >>> + } >> >>> >> >> can this be moved inside bxt_ddi_pll_select ? we can avoid this check for >> >> other platforms that also execute this function. >> > I asked Mika to do it this way, but if you feel strongly about it I >> > guess I could be persuaded otherwise too. >> > >> > My main point is, if we pass on DSI encoders to DDI functions in some >> > cases but mostly not, it will muddy the waters and eventually people end >> > up checking for "is dsi" all around DDI just because they can't be >> > bothered to check if the functions are really called for DDI only or >> > not. It's more of a maintainability concern than anything else. >> > >> > BR, >> > Jani. >> > >> i am fine with this either way. i was thinking of avoid such checks >> in other platforms where it is not needed but your concern of >> too many is_dsi checks is valid as well. >> with that i am fine with this change as is. >> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > > Another idea would be to use the clock_set thing to skip it, but > I think historically that has only been used to skip the PLL > calculations, not the PLL selection. So might be it would just confuse > things more. I just pushed this one. Thanks for the patch and review. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-02-09 15:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-05 11:29 [PATCH] drm/i915: Skip DDI PLL selection for DSI Mika Kahola 2016-02-05 11:47 ` ✗ Fi.CI.BAT: failure for " Patchwork 2016-02-09 2:52 ` [PATCH] " Thulasimani, Sivakumar 2016-02-09 6:32 ` Jani Nikula 2016-02-09 7:46 ` Thulasimani, Sivakumar 2016-02-09 9:52 ` Ville Syrjälä 2016-02-09 15:43 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).