Messages in this thread | | | Date | Thu, 05 Jul 2012 00:00:05 +0800 | From | "Yan, Zheng" <> | Subject | Re: [PATCH 5/5] perf/x86: Add Intel Nehalem-EX uncore support |
| |
On 07/04/2012 06:04 PM, Peter Zijlstra wrote: > On Wed, 2012-07-04 at 14:00 +0800, Yan, Zheng wrote: >> +static void nhmex_uncore_msr_enable_event(struct intel_uncore_box *box, >> + struct perf_event *event) >> +{ >> + struct hw_perf_event *hwc = &event->hw; >> + >> + if (box->pmu->type == &nhmex_uncore_bbox) >> + nhmex_bbox_msr_enable_event(box, event); >> + else if (box->pmu->type == &nhmex_uncore_sbox) >> + nhmex_sbox_msr_enable_event(box, event); >> + else if (box->pmu->type == &nhmex_uncore_mbox) >> + nhmex_mbox_msr_enable_event(box, event); >> + else if (box->pmu->type == &nhmex_uncore_rbox) >> + nhmex_rbox_msr_enable_event(box, event); >> + else if (hwc->idx >= UNCORE_PMC_IDX_FIXED) >> + wrmsrl(hwc->config_base, NHMEX_PMON_CTL_EN_BIT0); >> + else if (box->pmu->type->event_mask & NHMEX_PMON_CTL_EN_BIT0) >> + wrmsrl(hwc->config_base, hwc->config | NHMEX_PMON_CTL_EN_BIT22); >> + else >> + wrmsrl(hwc->config_base, hwc->config | NHMEX_PMON_CTL_EN_BIT0); >> +} > > wouldn't it be easier to do something like: > > box->pmu->type->enable_event(box, event); > Ok, I will update the patch.
> The same for these other functions that are massive ->type switches. >
> Also, can you please add more comments, note all the face_cpuc stuff, > and there's some rather dense code in all the alternative stuff. > OK
> Also, how can a single extra register require 192 bits of config space? > Some events in the M-Box and R-Box require programming up to 3 extra registers. For example, the event 0x5 in the R-Box requires programming extra registers XBR_SET1_MM_CFG, XBR_SET1_MATCH and XBR_SET1_MATCH.
Regards Yan, Zheng
| |